LinuxLists.cc - [RFC/PATCH v6 00/12] Media controller (core and V4L2)

[permalink] [raw]

Subject: [RFC/PATCH v6 01/12] media: Media device node support

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 <[email protected]>
---
drivers/media/Kconfig | 13 ++
drivers/media/Makefile | 10 +-
drivers/media/media-devnode.c | 321 +++++++++++++++++++++++++++++++++++++++++
include/media/media-devnode.h | 97 +++++++++++++
4 files changed, 439 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/media-devnode.c
create mode 100644 include/media/media-devnode.h

diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index a28541b..6b946e6 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -14,6 +14,19 @@ if MEDIA_SUPPORT
comment "Multimedia core support"

#
+# Media controller
+#
+
+config MEDIA_CONTROLLER
+ bool "Media Controller API (EXPERIMENTAL)"
+ depends on EXPERIMENTAL
+ ---help---
+ Enable the media controller API used to query media devices internal
+ topology and configure it dynamically.
+
+ This API is mostly used by camera interfaces in embedded platforms.
+
+#
# V4L core and enabled API's
#

diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 499b081..3a08991 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,7 +2,13 @@
# Makefile for the kernel multimedia device drivers.
#

+media-objs := media-devnode.o
+
+ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
+ obj-$(CONFIG_MEDIA_SUPPORT) += media.o
+endif
+
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..7804b70
--- /dev/null
+++ b/drivers/media/media-devnode.c
@@ -0,0 +1,321 @@
+/*
+ * Media device node
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Based on drivers/media/video/v4l2_dev.c code authored by
+ * Mauro Carvalho Chehab <[email protected]> (version 2)
+ * Alan Cox, <[email protected]> (version 1)
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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
+ *
+ * --
+ *
+ * Generic media device node infrastructure to register and unregister
+ * character devices using a dynamic major number and proper reference
+ * counting.
+ */
+
+#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;
+
+/*
+ * Active devices
+ */
+static DEFINE_MUTEX(media_devnode_lock);
+static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
+
+/* Called when the last user of the media device exits. */
+static void media_devnode_release(struct device *cd)
+{
+ struct media_devnode *mdev = to_media_devnode(cd);
+
+ mutex_lock(&media_devnode_lock);
+
+ /* Delete the cdev on this minor as well */
+ cdev_del(&mdev->cdev);
+
+ /* Mark device node number as free */
+ clear_bit(mdev->minor, media_devnode_nums);
+
+ mutex_unlock(&media_devnode_lock);
+
+ /* Release media_devnode and perform other cleanups as needed. */
+ if (mdev->release)
+ mdev->release(mdev);
+}
+
+static struct bus_type media_bus_type = {
+ .name = MEDIA_NAME,
+};
+
+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 (!media_devnode_is_registered(mdev))
+ return POLLERR | POLLHUP;
+ if (!mdev->fops->poll)
+ return DEFAULT_POLLMASK;
+ return mdev->fops->poll(filp, poll);
+}
+
+static long media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ struct media_devnode *mdev = media_devnode_data(filp);
+
+ if (!mdev->fops->ioctl)
+ return -ENOTTY;
+
+ if (!media_devnode_is_registered(mdev))
+ return -EIO;
+
+ return mdev->fops->ioctl(filp, cmd, arg);
+}
+
+/* Override for the open function */
+static int media_open(struct inode *inode, struct file *filp)
+{
+ struct media_devnode *mdev;
+ int ret;
+
+ /* Check if the media device is available. This needs to be done with
+ * the media_devnode_lock held to prevent an open/unregister race:
+ * without the lock, the device could be unregistered and freed between
+ * the media_devnode_is_registered() and get_device() calls, leading to
+ * a crash.
+ */
+ mutex_lock(&media_devnode_lock);
+ mdev = container_of(inode->i_cdev, struct media_devnode, cdev);
+ /* return ENXIO if the media device has been removed
+ already or if it is not registered anymore. */
+ if (!media_devnode_is_registered(mdev)) {
+ mutex_unlock(&media_devnode_lock);
+ return -ENXIO;
+ }
+ /* and increase the device refcount */
+ get_device(&mdev->dev);
+ mutex_unlock(&media_devnode_lock);
+
+ filp->private_data = mdev;
+
+ if (mdev->fops->open) {
+ ret = mdev->fops->open(filp);
+ if (ret) {
+ put_device(&mdev->dev);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/* Override for the release function */
+static int media_release(struct inode *inode, struct file *filp)
+{
+ struct media_devnode *mdev = media_devnode_data(filp);
+ int ret = 0;
+
+ if (mdev->fops->release)
+ mdev->fops->release(filp);
+
+ /* decrease the refcount unconditionally since the release()
+ return value is ignored. */
+ put_device(&mdev->dev);
+ filp->private_data = NULL;
+ return ret;
+}
+
+static const struct file_operations media_devnode_fops = {
+ .owner = THIS_MODULE,
+ .read = media_read,
+ .write = media_write,
+ .open = media_open,
+ .unlocked_ioctl = media_ioctl,
+ .release = media_release,
+ .poll = media_poll,
+ .llseek = no_llseek,
+};
+
+/**
+ * media_devnode_register - register a media device node
+ * @mdev: media device node structure we want to register
+ *
+ * The registration code assigns minor numbers and registers the new device node
+ * with the kernel. An error is returned if no free minor number can be found,
+ * or if the registration of the device node fails.
+ *
+ * Zero is returned on success.
+ *
+ * Note that if the media_devnode_register call fails, the release() callback of
+ * the media_devnode structure is *not* called, so the caller is responsible for
+ * freeing any data.
+ */
+int __must_check media_devnode_register(struct media_devnode *mdev)
+{
+ int minor;
+ int ret;
+
+ /* Part 1: Find a free minor number */
+ mutex_lock(&media_devnode_lock);
+ minor = find_next_zero_bit(media_devnode_nums, 0, MEDIA_NUM_DEVICES);
+ if (minor == MEDIA_NUM_DEVICES) {
+ mutex_unlock(&media_devnode_lock);
+ printk(KERN_ERR "could not get a free minor\n");
+ return -ENFILE;
+ }
+
+ set_bit(mdev->minor, media_devnode_nums);
+ mutex_unlock(&media_devnode_lock);
+
+ mdev->minor = minor;
+
+ /* Part 2: Initialize and register the character device */
+ cdev_init(&mdev->cdev, &media_devnode_fops);
+ mdev->cdev.owner = mdev->fops->owner;
+
+ ret = cdev_add(&mdev->cdev, MKDEV(MAJOR(media_dev_t), mdev->minor), 1);
+ if (ret < 0) {
+ printk(KERN_ERR "%s: cdev_add failed\n", __func__);
+ goto error;
+ }
+
+ /* Part 3: Register the media device */
+ mdev->dev.bus = &media_bus_type;
+ 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);
+ clear_bit(mdev->minor, media_devnode_nums);
+ 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 allocate major\n");
+ return ret;
+ }
+
+ ret = bus_register(&media_bus_type);
+ if (ret < 0) {
+ unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
+ printk(KERN_WARNING "media: bus_register failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static void __exit media_devnode_exit(void)
+{
+ bus_unregister(&media_bus_type);
+ unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
+}
+
+module_init(media_devnode_init)
+module_exit(media_devnode_exit)
+
+MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
+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..01cd034
--- /dev/null
+++ b/include/media/media-devnode.h
@@ -0,0 +1,97 @@
+/*
+ * Media device node
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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
+ *
+ * --
+ *
+ * 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);
+ int (*open) (struct file *);
+ int (*release) (struct file *);
+};
+
+/**
+ * struct media_devnode - Media device node
+ * @parent: parent device
+ * @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.
+ */
+struct media_devnode {
+ /* device ops */
+ const struct media_file_operations *fops;
+
+ /* sysfs */
+ struct device dev; /* media device */
+ struct cdev cdev; /* character device */
+ struct device *parent; /* device parent */
+
+ /* device info */
+ 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.2.2

2010-11-25 02:28:54

[permalink] [raw]

Subject: [RFC/PATCH v6 02/12] media: Media device

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 <[email protected]>
---
Documentation/DocBook/media-entities.tmpl | 2 +
Documentation/DocBook/media.tmpl | 3 +
Documentation/DocBook/v4l/media-controller.xml | 56 +++++++++++++
Documentation/media-framework.txt | 68 ++++++++++++++++
drivers/media/Makefile | 2 +-
drivers/media/media-device.c | 100 ++++++++++++++++++++++++
include/media/media-device.h | 66 ++++++++++++++++
7 files changed, 296 insertions(+), 1 deletions(-)
create mode 100644 Documentation/DocBook/v4l/media-controller.xml
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/DocBook/media-entities.tmpl b/Documentation/DocBook/media-entities.tmpl
index be34dcb..61d6f11 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -321,6 +321,8 @@
<!ENTITY sub-media-entities SYSTEM "media-entities.tmpl">
<!ENTITY sub-media-indices SYSTEM "media-indices.tmpl">

+<!ENTITY sub-media-controller SYSTEM "v4l/media-controller.xml">
+

<!ENTITY close SYSTEM "v4l/func-close.xml">
<!ENTITY ioctl SYSTEM "v4l/func-ioctl.xml">
diff --git a/Documentation/DocBook/media.tmpl b/Documentation/DocBook/media.tmpl
index f11048d..73464b0 100644
--- a/Documentation/DocBook/media.tmpl
+++ b/Documentation/DocBook/media.tmpl
@@ -106,6 +106,9 @@ Foundation. A copy of the license is included in the chapter entitled
&sub-remote_controllers;
</chapter>
</part>
+<part id="media_common">
+&sub-media-controller;
+</part>

&sub-fdl-appendix;

diff --git a/Documentation/DocBook/v4l/media-controller.xml b/Documentation/DocBook/v4l/media-controller.xml
new file mode 100644
index 0000000..253ddb4
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-controller.xml
@@ -0,0 +1,56 @@
+<partinfo>
+ <authorgroup>
+ <author>
+ <firstname>Laurent</firstname>
+ <surname>Pinchart</surname>
+ <affiliation><address><email>[email protected]</email></address></affiliation>
+ <contrib>Initial version.</contrib>
+ </author>
+ </authorgroup>
+ <copyright>
+ <year>2010</year>
+ <holder>Laurent Pinchart</holder>
+ </copyright>
+
+ <revhistory>
+ 
+ <revision>
+ <revnumber>1.0.0</revnumber>
+ <date>2010-11-10</date>
+ <authorinitials>lp</authorinitials>
+ <revremark>Initial revision</revremark>
+ </revision>
+ </revhistory>
+</partinfo>
+
+<title>Media Controller API</title>
+
+<chapter id="media_controller">
+ <title>Media Controller</title>
+
+ <section id="media-controller-intro">
+ <title>Introduction</title>
+ <para>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.</para>
+ <para>Independent functions, even when implemented in the same hardware, can
+ be modelled as 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.</para>
+ <para>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.</para>
+ <para>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.</para>
+ <para>The media controller API aims at solving those problems.</para>
+ </section>
+</chapter>
diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
new file mode 100644
index 0000000..84fa43a
--- /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
+------------
+
+The media controller API is documented in DocBook format in
+Documentation/DocBook/v4l/media-controller.xml. This document will focus on
+the kernel-side implementation of the media framework.
+
+
+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 must point to the parent device (usually a pci_dev, usb_interface or
+ platform_device instance).
+
+ - model must be filled with the device model name as a NUL-terminated UTF-8
+ string. The device/model revision must not be stored in this field.
+
+The following fields are optional:
+
+ - serial is a unique serial number stored as a NUL-terminated ASCII string.
+ The field is big enough to store a GUID in text form. If the hardware
+ doesn't provide a unique serial number this field must be left empty.
+
+ - bus_info represents the location of the device in the system as a
+ NUL-terminated ASCII string. For PCI/PCIe devices bus_info must be set to
+ "PCI:" (or "PCIe:") followed by the value of pci_name(). For USB devices,
+ the usb_make_path() function must be used. This field is used by
+ applications to distinguish between otherwise identical devices that don't
+ provide a serial number.
+
+ - hw_revision is the hardware device revision in a driver-specific format.
+ When possible the revision should be formatted with the KERNEL_VERSION
+ macro.
+
+ - driver_version is formatted with the KERNEL_VERSION macro. The version
+ minor must be incremented when new features are added to the userspace API
+ without breaking binary compatibility. The version major must be
+ incremented when binary compatibility is broken.
+
+Upon successful registration a character device named media[0-9]+ is created.
+The device major and minor numbers are dynamic. The model name is exported as
+a sysfs attribute.
+
+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 3a08991..019d3e0 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

ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
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..57a9c6b
--- /dev/null
+++ b/drivers/media/media-device.c
@@ -0,0 +1,100 @@
+/*
+ * Media device
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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,
+};
+
+/* -----------------------------------------------------------------------------
+ * sysfs
+ */
+
+static ssize_t show_model(struct device *cd,
+ struct device_attribute *attr, char *buf)
+{
+ struct media_device *mdev = to_media_device(to_media_devnode(cd));
+
+ return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
+}
+
+static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
+
+/* -----------------------------------------------------------------------------
+ * Registration/unregistration
+ */
+
+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 must point to the parent device
+ * - model must be filled with the device model name
+ */
+int __must_check media_device_register(struct media_device *mdev)
+{
+ int ret;
+
+ if (WARN_ON(mdev->dev == NULL || mdev->model[0] == 0))
+ return -EINVAL;
+
+ /* Register the device node. */
+ mdev->devnode.fops = &media_device_fops;
+ mdev->devnode.parent = mdev->dev;
+ mdev->devnode.release = media_device_release;
+ ret = media_devnode_register(&mdev->devnode);
+ if (ret < 0)
+ return ret;
+
+ ret = device_create_file(&mdev->devnode.dev, &dev_attr_model);
+ if (ret < 0) {
+ media_devnode_unregister(&mdev->devnode);
+ return ret;
+ }
+
+ return 0;
+}
+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)
+{
+ device_remove_file(&mdev->devnode.dev, &dev_attr_model);
+ 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..930a17a
--- /dev/null
+++ b/include/media/media-device.h
@@ -0,0 +1,66 @@
+/*
+ * Media device
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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>
+
+/**
+ * struct media_device - Media device
+ * @dev: Parent device
+ * @devnode: Media device node
+ * @model: Device model name
+ * @serial: Device serial number (optional)
+ * @bus_info: Unique and stable device location identifier
+ * @hw_revision: Hardware device revision
+ * @driver_version: Device driver version
+ *
+ * This structure represents an abstract high-level media device. It allows easy
+ * access to entities and provides basic media device-level support. The
+ * structure can be allocated directly or embedded in a larger structure.
+ *
+ * The parent @dev is a physical device. It must be set before registering the
+ * media device.
+ *
+ * @model is a descriptive model name exported through sysfs. It doesn't have to
+ * be unique.
+ */
+struct media_device {
+ /* dev->driver_data points to this struct. */
+ struct device *dev;
+ struct media_devnode devnode;
+
+ u8 model[32];
+ u8 serial[40];
+ u8 bus_info[32];
+ u32 hw_revision;
+ u32 driver_version;
+};
+
+int __must_check media_device_register(struct media_device *mdev);
+void media_device_unregister(struct media_device *mdev);
+
+#endif
--
1.7.2.2

2010-11-25 02:29:00

[permalink] [raw]

Subject: [RFC/PATCH v6 03/12] media: Entities, pads and links

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 <[email protected]> initial work.

Signed-off-by: Laurent Pinchart <[email protected]>
Signed-off-by: Sakari Ailus <[email protected]>
---
Documentation/DocBook/v4l/media-controller.xml | 20 +++
Documentation/media-framework.txt | 149 ++++++++++++++++++++++++
drivers/media/Makefile | 2 +-
drivers/media/media-device.c | 53 +++++++++
drivers/media/media-entity.c | 147 +++++++++++++++++++++++
include/media/media-device.h | 19 +++
include/media/media-entity.h | 118 +++++++++++++++++++
7 files changed, 507 insertions(+), 1 deletions(-)
create mode 100644 drivers/media/media-entity.c
create mode 100644 include/media/media-entity.h

diff --git a/Documentation/DocBook/v4l/media-controller.xml b/Documentation/DocBook/v4l/media-controller.xml
index 253ddb4..f89228d 100644
--- a/Documentation/DocBook/v4l/media-controller.xml
+++ b/Documentation/DocBook/v4l/media-controller.xml
@@ -53,4 +53,24 @@
implementing policies that belong to userspace.</para>
<para>The media controller API aims at solving those problems.</para>
</section>
+
+ <section id="media-controller-model">
+ <title>Media device model</title>
+ <para>Discovering a device internal topology, and configuring it at runtime,
+ is one of the goals of the media controller API. To achieve this, hardware
+ devices are modelled as an oriented graph of building blocks called entities
+ connected through pads.</para>
+ <para>An entity is a basic media hardware or software 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.</para>
+ <para>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.</para>
+ <para>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.</para>
+ </section>
</chapter>
diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 84fa43a..0332162 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -13,6 +13,30 @@ Documentation/DocBook/v4l/media-controller.xml. This document will focus on
the kernel-side implementation of the media framework.

+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,128 @@ 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, u16 num_pads,
+ struct media_pad *pads, u16 extra_links);
+
+The media_entity name, type, flags, revision and group_id fields can be
+initialized before or after calling media_entity_init. Entities embedded in
+higher-level standard structures can have some of 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.
+
+Entities have flags that describe the entity capabilities and state.
+
+ MEDIA_ENTITY_FLAG_DEFAULT indicates the default entity for a given
+ type. This can be used to report the default audio and video devices
+ or the default camera sensor.
+
+Logical entity groups can be defined by setting the group ID of all member
+entities to the same non-zero value. An entity group serves no purpose in the
+kernel, but is reported to userspace during entities enumeration. The group_id
+field belongs to the media device driver and must not by touched by entity
+drivers.
+
+Media device drivers should define groups if several entities are logically
+bound together. Example usages include reporting
+
+ - ALSA, VBI and video nodes that carry the same media stream
+ - lens and flash controllers associated with a sensor
+
+- Pads
+
+Pads are represented by a struct media_pad instance, defined in
+include/media/media-entity.h. Each entity stores its pads in a pads array
+managed by the entity driver. Drivers usually embed the array in a
+driver-specific structure.
+
+Pads are identified by their entity and their 0-based index in the pads array.
+Both information are stored in the media_pad structure, making the media_pad
+pointer the canonical way to store and pass link references.
+
+Pads have flags that describe the pad capabilities and state.
+
+ MEDIA_PAD_FLAG_INPUT indicates that the pad supports sinking data.
+ MEDIA_PAD_FLAG_OUTPUT indicates that the pad supports sourcing data.
+
+One and only one of MEDIA_PAD_FLAG_INPUT and MEDIA_PAD_FLAG_OUTPUT must be set
+for each pad.
+
+- Links
+
+Links are represented by a struct media_link instance, defined in
+include/media/media-entity.h. Each entity stores all links originating at or
+targetting any of its pads in a links array. A given link is thus stored
+twice, once in the source entity and once in the target entity. The array is
+pre-allocated and grows dynamically as needed.
+
+Drivers create links by calling
+
+ media_entity_create_link(struct media_entity *source, u16 source_pad,
+ struct media_entity *sink, u16 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 019d3e0..b890248 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

ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
obj-$(CONFIG_MEDIA_SUPPORT) += media.o
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 57a9c6b..2163610 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -25,6 +25,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,
@@ -69,6 +70,10 @@ int __must_check media_device_register(struct media_device *mdev)
if (WARN_ON(mdev->dev == NULL || mdev->model[0] == 0))
return -EINVAL;

+ mdev->entity_id = 1;
+ INIT_LIST_HEAD(&mdev->entities);
+ spin_lock_init(&mdev->lock);
+
/* Register the device node. */
mdev->devnode.fops = &media_device_fops;
mdev->devnode.parent = mdev->dev;
@@ -94,7 +99,55 @@ 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);
+
device_remove_file(&mdev->devnode.dev, &dev_attr_model);
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..e4ba2bc
--- /dev/null
+++ b/drivers/media/media-entity.c
@@ -0,0 +1,147 @@
+/*
+ * Media entity
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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, u16 num_pads,
+ struct media_pad *pads, u16 extra_links)
+{
+ struct media_link *links;
+ unsigned int max_links = num_pads + extra_links;
+ unsigned int i;
+
+ links = kzalloc(max_links * sizeof(links[0]), GFP_KERNEL);
+ if (links == NULL)
+ return -ENOMEM;
+
+ entity->group_id = 0;
+ 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_GPL(media_entity_init);
+
+void
+media_entity_cleanup(struct media_entity *entity)
+{
+ kfree(entity->links);
+}
+EXPORT_SYMBOL_GPL(media_entity_cleanup);
+
+static struct media_link *media_entity_add_link(struct media_entity *entity)
+{
+ if (entity->num_links >= entity->max_links) {
+ struct media_link *links = entity->links;
+ unsigned int max_links = entity->max_links + 2;
+ unsigned int i;
+
+ links = krealloc(links, max_links * sizeof(*links), GFP_KERNEL);
+ if (links == NULL)
+ return NULL;
+
+ for (i = 0; i < entity->num_links; i++)
+ links[i].reverse->reverse = &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, u16 source_pad,
+ struct media_entity *sink, u16 sink_pad, u32 flags)
+{
+ struct media_link *link;
+ struct media_link *backlink;
+
+ BUG_ON(source == NULL || sink == NULL);
+ BUG_ON(source_pad >= source->num_pads);
+ BUG_ON(sink_pad >= sink->num_pads);
+
+ link = media_entity_add_link(source);
+ if (link == NULL)
+ return -ENOMEM;
+
+ link->source = &source->pads[source_pad];
+ link->sink = &sink->pads[sink_pad];
+ link->flags = flags;
+
+ /* Create the backlink. Backlinks are used to help graph traversal and
+ * are not reported to userspace.
+ */
+ backlink = media_entity_add_link(sink);
+ if (backlink == NULL) {
+ source->num_links--;
+ return -ENOMEM;
+ }
+
+ backlink->source = &source->pads[source_pad];
+ backlink->sink = &sink->pads[sink_pad];
+ backlink->flags = flags;
+
+ link->reverse = backlink;
+ backlink->reverse = link;
+
+ sink->num_backlinks++;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(media_entity_create_link);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 930a17a..acd6a29 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -25,8 +25,10 @@

#include <linux/device.h>
#include <linux/list.h>
+#include <linux/spinlock.h>

#include <media/media-devnode.h>
+#include <media/media-entity.h>

/**
* struct media_device - Media device
@@ -37,6 +39,9 @@
* @bus_info: Unique and stable device location identifier
* @hw_revision: Hardware device revision
* @driver_version: Device driver version
+ * @entity_id: ID of the next entity to be registered
+ * @entities: List of registered entities
+ * @lock: Entities list lock
*
* This structure represents an abstract high-level media device. It allows easy
* access to entities and provides basic media device-level support. The
@@ -58,9 +63,23 @@ struct media_device {
u8 bus_info[32];
u32 hw_revision;
u32 driver_version;
+
+ u32 entity_id;
+ struct list_head entities;
+
+ /* Protects the entities list */
+ spinlock_t lock;
};

int __must_check media_device_register(struct media_device *mdev);
void media_device_unregister(struct media_device *mdev);

+int __must_check media_device_register_entity(struct media_device *mdev,
+ struct media_entity *entity);
+void media_device_unregister_entity(struct media_entity *entity);
+
+/* Iterate over all entities. */
+#define media_device_for_each_entity(entity, mdev) \
+ list_for_each_entry(entity, &(mdev)->entities, list)
+
#endif
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
new file mode 100644
index 0000000..60b4f09
--- /dev/null
+++ b/include/media/media-entity.h
@@ -0,0 +1,118 @@
+/*
+ * Media entity
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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_ENTITY_H
+#define _MEDIA_ENTITY_H
+
+#include <linux/list.h>
+
+#define MEDIA_ENTITY_TYPE_SHIFT 16
+#define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
+#define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
+
+#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV (2 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_SUBDEV_SENSOR (MEDIA_ENTITY_TYPE_SUBDEV + 1)
+#define MEDIA_ENTITY_TYPE_SUBDEV_FLASH (MEDIA_ENTITY_TYPE_SUBDEV + 2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_LENS (MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_ENTITY_FLAG_DEFAULT (1 << 0)
+
+#define MEDIA_LINK_FLAG_ACTIVE (1 << 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE (1 << 1)
+
+#define MEDIA_PAD_FLAG_INPUT (1 << 0)
+#define MEDIA_PAD_FLAG_OUTPUT (1 << 1)
+
+struct media_link {
+ struct media_pad *source; /* Source pad */
+ struct media_pad *sink; /* Sink pad */
+ struct media_link *reverse; /* Link in the reverse direction */
+ unsigned long flags; /* Link flags (MEDIA_LINK_FLAG_*) */
+};
+
+struct media_pad {
+ struct media_entity *entity; /* Entity this pad belongs to */
+ u16 index; /* Pad index in the entity pads array */
+ unsigned long flags; /* Pad flags (MEDIA_PAD_FLAG_*) */
+};
+
+struct media_entity {
+ struct list_head list;
+ struct media_device *parent; /* Media device this entity belongs to*/
+ u32 id; /* Entity ID, unique in the parent media
+ * device context */
+ const char *name; /* Entity name */
+ u32 type; /* Entity type (MEDIA_ENTITY_TYPE_*) */
+ u32 revision; /* Entity revision, driver specific */
+ unsigned long flags; /* Entity flags (MEDIA_ENTITY_FLAG_*) */
+ u32 group_id; /* Entity group ID */
+
+ u16 num_pads; /* Number of input and output pads */
+ u16 num_links; /* Number of existing links, both active
+ * and inactive */
+ u16 num_backlinks; /* Number of backlinks */
+ u16 max_links; /* Maximum number of links */
+
+ struct media_pad *pads; /* Pads array (num_pads elements) */
+ struct media_link *links; /* Links array (max_links elements)*/
+
+ union {
+ /* Node specifications */
+ struct {
+ u32 major;
+ u32 minor;
+ } v4l;
+ struct {
+ u32 major;
+ u32 minor;
+ } fb;
+ int alsa;
+ int dvb;
+
+ /* Sub-device specifications */
+ /* Nothing needed yet */
+ };
+};
+
+static inline u32 media_entity_type(struct media_entity *entity)
+{
+ return entity->type & MEDIA_ENTITY_TYPE_MASK;
+}
+
+static inline u32 media_entity_subtype(struct media_entity *entity)
+{
+ return entity->type & MEDIA_ENTITY_SUBTYPE_MASK;
+}
+
+int media_entity_init(struct media_entity *entity, u16 num_pads,
+ struct media_pad *pads, u16 extra_links);
+void media_entity_cleanup(struct media_entity *entity);
+int media_entity_create_link(struct media_entity *source, u16 source_pad,
+ struct media_entity *sink, u16 sink_pad, u32 flags);
+
+#endif
--
1.7.2.2

2010-11-25 02:29:40

[permalink] [raw]

Subject: [RFC/PATCH v6 04/12] media: Entity graph traversal

From: Sakari Ailus <[email protected]>

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 <[email protected]>
Signed-off-by: Laurent Pinchart <[email protected]>
Signed-off-by: Vimarsh Zutshi <[email protected]>
---
Documentation/media-framework.txt | 42 +++++++++++++
drivers/media/media-entity.c | 115 +++++++++++++++++++++++++++++++++++++
include/media/media-entity.h | 15 +++++
3 files changed, 172 insertions(+), 0 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 0332162..27a38a1 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -215,3 +215,45 @@ 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. To prevent infinite loops, the graph
+traversal code limits the maximum depth to MEDIA_ENTITY_ENUM_MAX_DEPTH,
+currently defined as 16.
+
+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 e4ba2bc..6230f74 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -84,6 +84,121 @@ media_entity_cleanup(struct media_entity *entity)
}
EXPORT_SYMBOL_GPL(media_entity_cleanup);

+/* -----------------------------------------------------------------------------
+ * Graph traversal
+ */
+
+static struct media_entity *
+media_entity_other(struct media_entity *entity, struct media_link *link)
+{
+ if (link->source->entity == entity)
+ return link->sink->entity;
+ else
+ return link->source->entity;
+}
+
+/* push an entity to traversal stack */
+static void stack_push(struct media_entity_graph *graph,
+ struct media_entity *entity)
+{
+ if (graph->top == MEDIA_ENTITY_ENUM_MAX_DEPTH - 1) {
+ WARN_ON(1);
+ return;
+ }
+ graph->top++;
+ graph->stack[graph->top].link = 0;
+ graph->stack[graph->top].entity = entity;
+}
+
+static struct media_entity *stack_pop(struct media_entity_graph *graph)
+{
+ struct media_entity *entity;
+
+ entity = graph->stack[graph->top].entity;
+ graph->top--;
+
+ return entity;
+}
+
+#define stack_peek(en) ((en)->stack[(en)->top - 1].entity)
+#define link_top(en) ((en)->stack[(en)->top].link)
+#define stack_top(en) ((en)->stack[(en)->top].entity)
+
+/**
+ * media_entity_graph_walk_start - Start walking the media graph at a given entity
+ * @graph: Media graph structure that will be used to walk the graph
+ * @entity: Starting entity
+ *
+ * This function initializes the graph traversal structure to walk the entities
+ * graph starting at the given entity. The traversal structure must not be
+ * modified by the caller during graph traversal. When done the structure can
+ * safely be freed.
+ */
+void media_entity_graph_walk_start(struct media_entity_graph *graph,
+ struct media_entity *entity)
+{
+ graph->top = 0;
+ graph->stack[graph->top].entity = NULL;
+ stack_push(graph, entity);
+}
+EXPORT_SYMBOL_GPL(media_entity_graph_walk_start);
+
+/**
+ * media_entity_graph_walk_next - Get the next entity in the graph
+ * @graph: Media graph structure
+ *
+ * Perform a depth-first traversal of the given media entities graph.
+ *
+ * The graph structure must have been previously initialized with a call to
+ * media_entity_graph_walk_start().
+ *
+ * Return the next entity in the graph or NULL if the whole graph have been
+ * traversed.
+ */
+struct media_entity *
+media_entity_graph_walk_next(struct media_entity_graph *graph)
+{
+ if (stack_top(graph) == NULL)
+ return NULL;
+
+ /*
+ * Depth first search. Push entity to stack and continue from
+ * top of the stack until no more entities on the level can be
+ * found.
+ */
+ while (link_top(graph) < stack_top(graph)->num_links) {
+ struct media_entity *entity = stack_top(graph);
+ struct media_link *link = &entity->links[link_top(graph)];
+ struct media_entity *next;
+
+ /* The link is not active so we do not follow. */
+ if (!(link->flags & MEDIA_LINK_FLAG_ACTIVE)) {
+ link_top(graph)++;
+ continue;
+ }
+
+ /* Get the entity in the other end of the link . */
+ next = media_entity_other(entity, link);
+
+ /* Was it the entity we came here from? */
+ if (next == stack_peek(graph)) {
+ link_top(graph)++;
+ continue;
+ }
+
+ /* Push the new entity to stack and start over. */
+ link_top(graph)++;
+ stack_push(graph, next);
+ }
+
+ return stack_pop(graph);
+}
+EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
+
+/* -----------------------------------------------------------------------------
+ * Links management
+ */
+
static struct media_link *media_entity_add_link(struct media_entity *entity)
{
if (entity->num_links >= entity->max_links) {
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 60b4f09..721f450 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -109,10 +109,25 @@ static inline u32 media_entity_subtype(struct media_entity *entity)
return entity->type & MEDIA_ENTITY_SUBTYPE_MASK;
}

+#define MEDIA_ENTITY_ENUM_MAX_DEPTH 16
+
+struct media_entity_graph {
+ struct {
+ struct media_entity *entity;
+ int link;
+ } stack[MEDIA_ENTITY_ENUM_MAX_DEPTH];
+ int top;
+};
+
int media_entity_init(struct media_entity *entity, u16 num_pads,
struct media_pad *pads, u16 extra_links);
void media_entity_cleanup(struct media_entity *entity);
int media_entity_create_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 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.2.2

2010-11-25 02:29:46

[permalink] [raw]

Subject: [RFC/PATCH v6 05/12] media: Reference count and power handling

From: Sakari Ailus <[email protected]>

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 <[email protected]>
Signed-off-by: Laurent Pinchart <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
---
Documentation/media-framework.txt | 37 +++++++++
drivers/media/media-device.c | 1 +
drivers/media/media-entity.c | 146 +++++++++++++++++++++++++++++++++++++
include/media/media-device.h | 4 +
include/media/media-entity.h | 15 ++++
5 files changed, 203 insertions(+), 0 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 27a38a1..9fdbc50 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -257,3 +257,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 2163610..52e5985 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -73,6 +73,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);

/* Register the device node. */
mdev->devnode.fops = &media_device_fops;
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 6230f74..c15f16b 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -23,6 +23,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
@@ -196,6 +197,151 @@ media_entity_graph_walk_next(struct media_entity_graph *graph)
EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);

/* -----------------------------------------------------------------------------
+ * Power state handling
+ */
+
+/* Apply use count to an entity. */
+static void media_entity_use_apply_one(struct media_entity *entity, int change)
+{
+ entity->use_count += change;
+ WARN_ON(entity->use_count < 0);
+}
+
+/*
+ * Apply use count change to an entity and change power state based on
+ * new use count.
+ */
+static int media_entity_power_apply_one(struct media_entity *entity, int change)
+{
+ int ret;
+
+ if (entity->use_count == 0 && change > 0 &&
+ entity->ops && entity->ops->set_power) {
+ ret = entity->ops->set_power(entity, 1);
+ if (ret)
+ return ret;
+ }
+
+ media_entity_use_apply_one(entity, change);
+
+ if (entity->use_count == 0 && change < 0 &&
+ entity->ops && entity->ops->set_power)
+ entity->ops->set_power(entity, 0);
+
+ return 0;
+}
+
+/*
+ * Apply power change to all connected entities. This ignores the
+ * nodes.
+ */
+static int media_entity_power_apply(struct media_entity *entity, int change)
+{
+ struct media_entity_graph graph;
+ struct media_entity *first = entity;
+ int ret = 0;
+
+ if (!change)
+ return 0;
+
+ media_entity_graph_walk_start(&graph, entity);
+
+ while (!ret && (entity = media_entity_graph_walk_next(&graph)))
+ if (media_entity_type(entity) != MEDIA_ENTITY_TYPE_NODE)
+ ret = media_entity_power_apply_one(entity, change);
+
+ if (!ret)
+ return 0;
+
+ media_entity_graph_walk_start(&graph, first);
+
+ while ((first = media_entity_graph_walk_next(&graph))
+ && first != entity)
+ if (media_entity_type(first) != MEDIA_ENTITY_TYPE_NODE)
+ media_entity_power_apply_one(first, -change);
+
+ return ret;
+}
+
+/*
+ * Apply use count change to graph and change power state of entities
+ * accordingly.
+ */
+static int media_entity_node_power_change(struct media_entity *entity,
+ int change)
+{
+ /* Apply use count to node. */
+ media_entity_use_apply_one(entity, change);
+
+ /* Apply power change to connected non-nodes. */
+ return media_entity_power_apply(entity, change);
+}
+
+/*
+ * Node entity use changes are reflected on power state of all
+ * connected (directly or indirectly) entities whereas non-node entity
+ * use count changes are limited to that very entity.
+ */
+static int media_entity_use_change(struct media_entity *entity, int change)
+{
+ if (media_entity_type(entity) == MEDIA_ENTITY_TYPE_NODE)
+ return media_entity_node_power_change(entity, change);
+ else
+ return media_entity_power_apply_one(entity, change);
+}
+
+static struct media_entity *__media_entity_get(struct media_entity *entity)
+{
+ if (media_entity_use_change(entity, 1))
+ return NULL;
+
+ return entity;
+}
+
+static void __media_entity_put(struct media_entity *entity)
+{
+ media_entity_use_change(entity, -1);
+}
+
+/* user open()s media entity */
+struct media_entity *media_entity_get(struct media_entity *entity)
+{
+ struct media_entity *e;
+
+ if (entity == NULL)
+ return NULL;
+
+ if (entity->parent->dev &&
+ !try_module_get(entity->parent->dev->driver->owner))
+ return NULL;
+
+ mutex_lock(&entity->parent->graph_mutex);
+ e = __media_entity_get(entity);
+ mutex_unlock(&entity->parent->graph_mutex);
+
+ if (e == NULL && entity->parent->dev)
+ module_put(entity->parent->dev->driver->owner);
+
+ return e;
+}
+EXPORT_SYMBOL_GPL(media_entity_get);
+
+/* user release()s media entity */
+void media_entity_put(struct media_entity *entity)
+{
+ if (entity == NULL)
+ return;
+
+ mutex_lock(&entity->parent->graph_mutex);
+ __media_entity_put(entity);
+ mutex_unlock(&entity->parent->graph_mutex);
+
+ if (entity->parent->dev)
+ module_put(entity->parent->dev->driver->owner);
+}
+EXPORT_SYMBOL_GPL(media_entity_put);
+
+/* -----------------------------------------------------------------------------
* Links management
*/

diff --git a/include/media/media-device.h b/include/media/media-device.h
index acd6a29..461cb29 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -25,6 +25,7 @@

#include <linux/device.h>
#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/spinlock.h>

#include <media/media-devnode.h>
@@ -42,6 +43,7 @@
* @entity_id: ID of the next entity to be registered
* @entities: List of registered entities
* @lock: Entities list lock
+ * @graph_mutex: Entities graph operation lock
*
* This structure represents an abstract high-level media device. It allows easy
* access to entities and provides basic media device-level support. The
@@ -69,6 +71,8 @@ struct media_device {

/* Protects the entities list */
spinlock_t lock;
+ /* Serializes graph operations. */
+ struct mutex graph_mutex;
};

int __must_check media_device_register(struct media_device *mdev);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 721f450..b9694b5 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -61,6 +61,10 @@ struct media_pad {
unsigned long flags; /* Pad flags (MEDIA_PAD_FLAG_*) */
};

+struct media_entity_operations {
+ int (*set_power)(struct media_entity *entity, int power);
+};
+
struct media_entity {
struct list_head list;
struct media_device *parent; /* Media device this entity belongs to*/
@@ -81,6 +85,10 @@ struct media_entity {
struct media_pad *pads; /* Pads array (num_pads elements) */
struct media_link *links; /* Links array (max_links elements)*/

+ const struct media_entity_operations *ops; /* Entity operations */
+
+ int use_count; /* Use count for the entity. */
+
union {
/* Node specifications */
struct {
@@ -125,9 +133,16 @@ void media_entity_cleanup(struct media_entity *entity);
int media_entity_create_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 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.2.2

2010-11-25 02:30:27

[permalink] [raw]

Subject: [RFC/PATCH v6 06/12] media: Media device information query

Create the following ioctl and implement it at the media device level to
query device information.

- MEDIA_IOC_DEVICE_INFO: Query media device information

The ioctl and its data structure are defined in the new kernel header
linux/media.h available to userspace applications.

Signed-off-by: Laurent Pinchart <[email protected]>
---
Documentation/DocBook/media-entities.tmpl | 12 ++
Documentation/DocBook/v4l/media-controller.xml | 10 ++
Documentation/DocBook/v4l/media-func-close.xml | 59 +++++++++
Documentation/DocBook/v4l/media-func-ioctl.xml | 116 +++++++++++++++++
Documentation/DocBook/v4l/media-func-open.xml | 94 ++++++++++++++
.../DocBook/v4l/media-ioc-device-info.xml | 132 ++++++++++++++++++++
drivers/media/media-device.c | 57 +++++++++
include/linux/Kbuild | 1 +
include/linux/media.h | 45 +++++++
include/media/media-device.h | 3 +
10 files changed, 529 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/media-func-close.xml
create mode 100644 Documentation/DocBook/v4l/media-func-ioctl.xml
create mode 100644 Documentation/DocBook/v4l/media-func-open.xml
create mode 100644 Documentation/DocBook/v4l/media-ioc-device-info.xml
create mode 100644 include/linux/media.h

diff --git a/Documentation/DocBook/media-entities.tmpl b/Documentation/DocBook/media-entities.tmpl
index 61d6f11..6af3375 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -11,6 +11,10 @@
<!ENTITY func-select "<link linkend='func-select'><function>select()</function></link>">
<!ENTITY func-write "<link linkend='func-write'><function>write()</function></link>">

+<!ENTITY media-func-close "<link linkend='media-func-close'><function>close()</function></link>">
+<!ENTITY media-func-ioctl "<link linkend='media-func-ioctl'><function>ioctl()</function></link>">
+<!ENTITY media-func-open "<link linkend='media-func-open'><function>open()</function></link>">
+

<!ENTITY VIDIOC-CROPCAP "<link linkend='vidioc-cropcap'><constant>VIDIOC_CROPCAP</constant></link>">
<!ENTITY VIDIOC-DBG-G-CHIP-IDENT "<link linkend='vidioc-dbg-g-chip-ident'><constant>VIDIOC_DBG_G_CHIP_IDENT</constant></link>">
@@ -87,6 +91,8 @@
<!ENTITY VIDIOC-TRY-FMT "<link linkend='vidioc-g-fmt'><constant>VIDIOC_TRY_FMT</constant></link>">
<!ENTITY VIDIOC-UNSUBSCRIBE-EVENT "<link linkend='vidioc-subscribe-event'><constant>VIDIOC_UNSUBSCRIBE_EVENT</constant></link>">

+<!ENTITY MEDIA-IOC-DEVICE-INFO "<link linkend='media-ioc-device-info'><constant>MEDIA_IOC_DEVICE_INFO</constant></link>">
+

<!ENTITY v4l2-std-id "<link linkend='v4l2-std-id'>v4l2_std_id</link>">

@@ -181,6 +187,8 @@
<!ENTITY v4l2-vbi-format "struct <link linkend='v4l2-vbi-format'>v4l2_vbi_format</link>">
<!ENTITY v4l2-window "struct <link linkend='v4l2-window'>v4l2_window</link>">

+<!ENTITY media-device-info "struct <link linkend='media-device-info'>media_device_info</link>">
+

<!ENTITY EACCES "<errorcode>EACCES</errorcode> error code">
<!ENTITY EAGAIN "<errorcode>EAGAIN</errorcode> error code">
@@ -322,6 +330,10 @@
<!ENTITY sub-media-indices SYSTEM "media-indices.tmpl">

<!ENTITY sub-media-controller SYSTEM "v4l/media-controller.xml">
+<!ENTITY sub-media-open SYSTEM "v4l/media-func-open.xml">
+<!ENTITY sub-media-close SYSTEM "v4l/media-func-close.xml">
+<!ENTITY sub-media-ioctl SYSTEM "v4l/media-func-ioctl.xml">
+<!ENTITY sub-media-ioc-device-info SYSTEM "v4l/media-ioc-device-info.xml">


<!ENTITY close SYSTEM "v4l/func-close.xml">
diff --git a/Documentation/DocBook/v4l/media-controller.xml b/Documentation/DocBook/v4l/media-controller.xml
index f89228d..a46b786 100644
--- a/Documentation/DocBook/v4l/media-controller.xml
+++ b/Documentation/DocBook/v4l/media-controller.xml
@@ -74,3 +74,13 @@
pad to a sink pad.</para>
</section>
</chapter>
+
+<appendix id="media-user-func">
+ <title>Function Reference</title>
+ 
+ &sub-media-open;
+ &sub-media-close;
+ &sub-media-ioctl;
+ 
+ &sub-media-ioc-device-info;
+</appendix>
diff --git a/Documentation/DocBook/v4l/media-func-close.xml b/Documentation/DocBook/v4l/media-func-close.xml
new file mode 100644
index 0000000..be149c8
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-func-close.xml
@@ -0,0 +1,59 @@
+<refentry id="media-func-close">
+ <refmeta>
+ <refentrytitle>media close()</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>media-close</refname>
+ <refpurpose>Close a media device</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcsynopsisinfo>#include <unistd.h></funcsynopsisinfo>
+ <funcprototype>
+ <funcdef>int <function>close</function></funcdef>
+ <paramdef>int <parameter>fd</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>fd</parameter></term>
+ <listitem>
+ <para>&fd;</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para>Closes the media device. Resources associated with the file descriptor
+ are freed. The device configuration remain unchanged.</para>
+ </refsect1>
+
+ <refsect1>
+ <title>Return Value</title>
+
+ <para><function>close</function> returns 0 on success. On error, -1 is
+ returned, and <varname>errno</varname> is set appropriately. Possible error
+ codes are:</para>
+
+ <variablelist>
+ <varlistentry>
+ <term><errorcode>EBADF</errorcode></term>
+ <listitem>
+ <para><parameter>fd</parameter> is not a valid open file descriptor.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/v4l/media-func-ioctl.xml b/Documentation/DocBook/v4l/media-func-ioctl.xml
new file mode 100644
index 0000000..bda8604
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-func-ioctl.xml
@@ -0,0 +1,116 @@
+<refentry id="media-func-ioctl">
+ <refmeta>
+ <refentrytitle>media ioctl()</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>media-ioctl</refname>
+ <refpurpose>Control a media device</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcsynopsisinfo>#include <sys/ioctl.h></funcsynopsisinfo>
+ <funcprototype>
+ <funcdef>int <function>ioctl</function></funcdef>
+ <paramdef>int <parameter>fd</parameter></paramdef>
+ <paramdef>int <parameter>request</parameter></paramdef>
+ <paramdef>void *<parameter>argp</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>fd</parameter></term>
+ <listitem>
+ <para>&fd;</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>request</parameter></term>
+ <listitem>
+ <para>Media ioctl request code as defined in the media.h header file,
+ for example MEDIA_IOC_SETUP_LINK.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>argp</parameter></term>
+ <listitem>
+ <para>Pointer to a request-specific structure.</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Description</title>
+ <para>The <function>ioctl()</function> function manipulates media device
+ parameters. The argument <parameter>fd</parameter> must be an open file
+ descriptor.</para>
+ <para>The ioctl <parameter>request</parameter> code specifies the media
+ function to be called. It has encoded in it whether the argument is an
+ input, output or read/write parameter, and the size of the argument
+ <parameter>argp</parameter> in bytes.</para>
+ <para>Macros and structures definitions specifying media ioctl requests and
+ their parameters are located in the media.h header file. All media ioctl
+ requests, their respective function and parameters are specified in
+ <xref linkend="media-user-func" />.</para>
+ </refsect1>
+
+ <refsect1>
+ <title>Return Value</title>
+
+ <para><function>ioctl()</function> returns <returnvalue>0</returnvalue> on
+ success. On failure, <returnvalue>-1</returnvalue> is returned, and the
+ <varname>errno</varname> variable is set appropriately. Generic error codes
+ are listed below, and request-specific error codes are listed in the
+ individual requests descriptions.</para>
+ <para>When an ioctl that takes an output or read/write parameter fails,
+ the parameter remains unmodified.</para>
+
+ <variablelist>
+ <varlistentry>
+ <term><errorcode>EBADF</errorcode></term>
+ <listitem>
+ <para><parameter>fd</parameter> is not a valid open file descriptor.
+ </para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>EFAULT</errorcode></term>
+ <listitem>
+ <para><parameter>argp</parameter> references an inaccessible memory
+ area.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>EINVAL</errorcode></term>
+ <listitem>
+ <para>The <parameter>request</parameter> or the data pointed to by
+ <parameter>argp</parameter> is not valid. This is a very common error
+ code, see the individual ioctl requests listed in
+ <xref linkend="media-user-func" /> for actual causes.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>ENOMEM</errorcode></term>
+ <listitem>
+ <para>Insufficient kernel memory was available to complete the
+ request.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>ENOTTY</errorcode></term>
+ <listitem>
+ <para><parameter>fd</parameter> is not associated with a character
+ special device.</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/v4l/media-func-open.xml b/Documentation/DocBook/v4l/media-func-open.xml
new file mode 100644
index 0000000..f7df034
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-func-open.xml
@@ -0,0 +1,94 @@
+<refentry id="media-func-open">
+ <refmeta>
+ <refentrytitle>media open()</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>media-open</refname>
+ <refpurpose>Open a media device</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcsynopsisinfo>#include <fcntl.h></funcsynopsisinfo>
+ <funcprototype>
+ <funcdef>int <function>open</function></funcdef>
+ <paramdef>const char *<parameter>device_name</parameter></paramdef>
+ <paramdef>int <parameter>flags</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>device_name</parameter></term>
+ <listitem>
+ <para>Device to be opened.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>flags</parameter></term>
+ <listitem>
+ <para>Open flags. Access mode must be either <constant>O_RDONLY</constant>
+ or <constant>O_RDWR</constant>. Other flags have no effect.</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+ <refsect1>
+ <title>Description</title>
+ <para>To open a media device applications call <function>open()</function>
+ with the desired device name. The function has no side effects; the device
+ configuration remain unchanged.</para>
+ <para>When the device is opened in read-only mode, attemps to modify its
+ configuration will result in an error, and <varname>errno</varname> will be
+ set to <errorcode>EBADF</errorcode>.</para>
+ </refsect1>
+ <refsect1>
+ <title>Return Value</title>
+
+ <para><function>open</function> returns the new file descriptor on success.
+ On error, -1 is returned, and <varname>errno</varname> is set appropriately.
+ Possible error codes are:</para>
+
+ <variablelist>
+ <varlistentry>
+ <term><errorcode>EACCES</errorcode></term>
+ <listitem>
+ <para>The requested access to the file is not allowed.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>EMFILE</errorcode></term>
+ <listitem>
+ <para>The process already has the maximum number of files open.
+ </para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>ENFILE</errorcode></term>
+ <listitem>
+ <para>The system limit on the total number of open files has been
+ reached.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>ENOMEM</errorcode></term>
+ <listitem>
+ <para>Insufficient kernel memory was available.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>ENXIO</errorcode></term>
+ <listitem>
+ <para>No device corresponding to this device special file exists.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/v4l/media-ioc-device-info.xml b/Documentation/DocBook/v4l/media-ioc-device-info.xml
new file mode 100644
index 0000000..ffd0fb8
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-ioc-device-info.xml
@@ -0,0 +1,132 @@
+<refentry id="media-ioc-device-info">
+ <refmeta>
+ <refentrytitle>ioctl MEDIA_IOC_DEVICE_INFO</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>MEDIA_IOC_DEVICE_INFO</refname>
+ <refpurpose>Query device information</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcprototype>
+ <funcdef>int <function>ioctl</function></funcdef>
+ <paramdef>int <parameter>fd</parameter></paramdef>
+ <paramdef>int <parameter>request</parameter></paramdef>
+ <paramdef>struct media_device_info *<parameter>argp</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>fd</parameter></term>
+ <listitem>
+ <para>&fd;</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>request</parameter></term>
+ <listitem>
+ <para>MEDIA_IOC_DEVICE_INFO</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>argp</parameter></term>
+ <listitem>
+ <para></para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para>All media devices must support the <constant>MEDIA_IOC_DEVICE_INFO</constant>
+ ioctl. To query device information, applications call the ioctl with a
+ pointer to a &media-device-info;. The driver fills the structure and returns
+ the information to the application.
+ The ioctl never fails.</para>
+
+ <table pgwide="1" frame="none" id="media-device-info">
+ <title>struct <structname>media_device_info</structname></title>
+ <tgroup cols="3">
+ &cs-str;
+ <tbody valign="top">
+ <row>
+ <entry>__u8</entry>
+ <entry><structfield>driver</structfield>[16]</entry>
+ <entry><para>Name of the driver implementing the media API as a
+ NUL-terminated ASCII string. The driver version is stored in the
+ <structfield>driver_version</structfield> field.</para>
+ <para>Driver specific applications can use this information to
+ verify the driver identity. It is also useful to work around
+ known bugs, or to identify drivers in error reports.</para></entry>
+ </row>
+ <row>
+ <entry>__u8</entry>
+ <entry><structfield>model</structfield>[32]</entry>
+ <entry>Device model name as a NUL-terminated UTF-8 string. The
+ device version is stored in the <structfield>device_version</structfield>
+ field and is not be appended to the model name.</entry>
+ </row>
+ <row>
+ <entry>__u8</entry>
+ <entry><structfield>serial</structfield>[40]</entry>
+ <entry>Serial number as a NUL-terminated ASCII string.</entry>
+ </row>
+ <row>
+ <entry>__u8</entry>
+ <entry><structfield>bus_info</structfield>[32]</entry>
+ <entry>Location of the device in the system as a NUL-terminated
+ ASCII string. This includes the bus type name (PCI, USB, ...) and a
+ bus-specific identifier.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>media_version</structfield></entry>
+ <entry>Media API version, formatted with the
+ <constant>KERNEL_VERSION()</constant> macro.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>hw_revision</structfield></entry>
+ <entry>Hardware device revision in a driver-specific format.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>media_version</structfield></entry>
+ <entry>Media device driver version, formatted with the
+ <constant>KERNEL_VERSION()</constant> macro. Together with the
+ <structfield>driver</structfield> field this identifies a particular
+ driver.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>reserved</structfield>[31]</entry>
+ <entry>Reserved for future extensions. Drivers and applications must
+ set this array to zero.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+ <para>The <structfield>serial</structfield> and <structfield>bus_info</structfield>
+ fields can be used to distinguish between multiple instances of otherwise
+ identical hardware. The serial number takes precedence when provided and can
+ be assumed to be unique. If the serial number is an empty string, the
+ <structfield>bus_info</structfield> field can be used instead. The
+ <structfield>bus_info</structfield> field is guaranteed to be unique, but
+ can vary across reboots or device unplug/replug.</para>
+ </refsect1>
+
+ <refsect1>
+ <title>Return value</title>
+ <para>This function doesn't return specific error codes.</para>
+ </refsect1>
+</refentry>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 52e5985..d02038a 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -22,13 +22,70 @@

#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>

+/* -----------------------------------------------------------------------------
+ * Userspace API
+ */
+
+static int media_device_open(struct file *filp)
+{
+ return 0;
+}
+
+static int media_device_close(struct file *filp)
+{
+ return 0;
+}
+
+static int media_device_get_info(struct media_device *dev,
+ struct media_device_info __user *__info)
+{
+ struct media_device_info info;
+
+ memset(&info, 0, sizeof(info));
+
+ strlcpy(info.driver, dev->dev->driver->name, sizeof(info.driver));
+ strlcpy(info.model, dev->model, sizeof(info.model));
+ strlcpy(info.serial, dev->serial, sizeof(info.serial));
+ strlcpy(info.bus_info, dev->bus_info, sizeof(info.bus_info));
+
+ info.media_version = MEDIA_API_VERSION;
+ info.hw_revision = dev->hw_revision;
+ info.driver_version = dev->driver_version;
+
+ return copy_to_user(__info, &info, sizeof(*__info));
+}
+
+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_DEVICE_INFO:
+ ret = media_device_get_info(dev,
+ (struct media_device_info __user *)arg);
+ break;
+
+ default:
+ ret = -ENOIOCTLCMD;
+ }
+
+ return ret;
+}
+
static const struct media_file_operations media_device_fops = {
.owner = THIS_MODULE,
+ .open = media_device_open,
+ .ioctl = media_device_ioctl,
+ .release = media_device_close,
};

/* -----------------------------------------------------------------------------
diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index 97319a8..26e0a7f 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -228,6 +228,7 @@ header-y += magic.h
header-y += major.h
header-y += map_to_7segment.h
header-y += matroxfb.h
+header-y += media.h
header-y += mempolicy.h
header-y += meye.h
header-y += mii.h
diff --git a/include/linux/media.h b/include/linux/media.h
new file mode 100644
index 0000000..8108fe9
--- /dev/null
+++ b/include/linux/media.h
@@ -0,0 +1,45 @@
+/*
+ * Multimedia device API
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contacts: Laurent Pinchart <[email protected]>
+ * Sakari Ailus <[email protected]>
+ *
+ * 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.
+ *
+ * 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 __LINUX_MEDIA_H
+#define __LINUX_MEDIA_H
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+#include <linux/version.h>
+
+#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
+
+struct media_device_info {
+ __u8 driver[16];
+ __u8 model[32];
+ __u8 serial[40];
+ __u8 bus_info[32];
+ __u32 media_version;
+ __u32 hw_revision;
+ __u32 driver_version;
+ __u32 reserved[31];
+};
+
+#define MEDIA_IOC_DEVICE_INFO _IOWR('M', 1, struct media_device_info)
+
+#endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 461cb29..bf2eb2c 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -75,6 +75,9 @@ struct media_device {
struct mutex graph_mutex;
};

+/* media_devnode to media_device */
+#define to_media_device(node) container_of(node, struct media_device, devnode)
+
int __must_check media_device_register(struct media_device *mdev);
void media_device_unregister(struct media_device *mdev);

--
1.7.2.2

2010-11-25 02:30:33

[permalink] [raw]

Subject: [RFC/PATCH v6 07/12] media: Entities, pads and links enumeration

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 <[email protected]>
Signed-off-by: Sakari Ailus <[email protected]>
---
Documentation/DocBook/media-entities.tmpl | 8 +
Documentation/DocBook/v4l/media-controller.xml | 2 +
.../DocBook/v4l/media-ioc-device-info.xml | 3 +-
.../DocBook/v4l/media-ioc-enum-entities.xml | 287 ++++++++++++++++++++
Documentation/DocBook/v4l/media-ioc-enum-links.xml | 202 ++++++++++++++
drivers/media/media-device.c | 123 +++++++++
include/linux/media.h | 81 ++++++
include/media/media-entity.h | 24 +--
8 files changed, 706 insertions(+), 24 deletions(-)
create mode 100644 Documentation/DocBook/v4l/media-ioc-enum-entities.xml
create mode 100644 Documentation/DocBook/v4l/media-ioc-enum-links.xml

diff --git a/Documentation/DocBook/media-entities.tmpl b/Documentation/DocBook/media-entities.tmpl
index 6af3375..6e7dae4 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -92,6 +92,8 @@
<!ENTITY VIDIOC-UNSUBSCRIBE-EVENT "<link linkend='vidioc-subscribe-event'><constant>VIDIOC_UNSUBSCRIBE_EVENT</constant></link>">

<!ENTITY MEDIA-IOC-DEVICE-INFO "<link linkend='media-ioc-device-info'><constant>MEDIA_IOC_DEVICE_INFO</constant></link>">
+<!ENTITY MEDIA-IOC-ENUM-ENTITIES "<link linkend='media-ioc-enum-entities'><constant>MEDIA_IOC_ENUM_ENTITIES</constant></link>">
+<!ENTITY MEDIA-IOC-ENUM-LINKS "<link linkend='media-ioc-enum-links'><constant>MEDIA_IOC_ENUM_LINKS</constant></link>">


<!ENTITY v4l2-std-id "<link linkend='v4l2-std-id'>v4l2_std_id</link>">
@@ -188,6 +190,10 @@
<!ENTITY v4l2-window "struct <link linkend='v4l2-window'>v4l2_window</link>">

<!ENTITY media-device-info "struct <link linkend='media-device-info'>media_device_info</link>">
+<!ENTITY media-entity-desc "struct <link linkend='media-entity-desc'>media_entity_desc</link>">
+<!ENTITY media-links-enum "struct <link linkend='media-links-enum'>media_links_enum</link>">
+<!ENTITY media-pad-desc "struct <link linkend='media-pad-desc'>media_pad_desc</link>">
+<!ENTITY media-link-desc "struct <link linkend='media-link-desc'>media_link_desc</link>">


<!ENTITY EACCES "<errorcode>EACCES</errorcode> error code">
@@ -334,6 +340,8 @@
<!ENTITY sub-media-close SYSTEM "v4l/media-func-close.xml">
<!ENTITY sub-media-ioctl SYSTEM "v4l/media-func-ioctl.xml">
<!ENTITY sub-media-ioc-device-info SYSTEM "v4l/media-ioc-device-info.xml">
+<!ENTITY sub-media-ioc-enum-entities SYSTEM "v4l/media-ioc-enum-entities.xml">
+<!ENTITY sub-media-ioc-enum-links SYSTEM "v4l/media-ioc-enum-links.xml">


<!ENTITY close SYSTEM "v4l/func-close.xml">
diff --git a/Documentation/DocBook/v4l/media-controller.xml b/Documentation/DocBook/v4l/media-controller.xml
index a46b786..2c4fd2b 100644
--- a/Documentation/DocBook/v4l/media-controller.xml
+++ b/Documentation/DocBook/v4l/media-controller.xml
@@ -83,4 +83,6 @@
&sub-media-ioctl;

&sub-media-ioc-device-info;
+ &sub-media-ioc-enum-entities;
+ &sub-media-ioc-enum-links;
</appendix>
diff --git a/Documentation/DocBook/v4l/media-ioc-device-info.xml b/Documentation/DocBook/v4l/media-ioc-device-info.xml
index ffd0fb8..aa25c31 100644
--- a/Documentation/DocBook/v4l/media-ioc-device-info.xml
+++ b/Documentation/DocBook/v4l/media-ioc-device-info.xml
@@ -27,7 +27,8 @@
<varlistentry>
<term><parameter>fd</parameter></term>
<listitem>
- <para>&fd;</para>
+ <para>File descriptor returned by
+ <link linkend='media-func-open'><function>open()</function></link>.</para>
</listitem>
</varlistentry>
<varlistentry>
diff --git a/Documentation/DocBook/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/v4l/media-ioc-enum-entities.xml
new file mode 100644
index 0000000..edb1acc
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-ioc-enum-entities.xml
@@ -0,0 +1,287 @@
+<refentry id="media-ioc-enum-entities">
+ <refmeta>
+ <refentrytitle>ioctl MEDIA_IOC_ENUM_ENTITIES</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>MEDIA_IOC_ENUM_ENTITIES</refname>
+ <refpurpose>Enumerate entities and their properties</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcprototype>
+ <funcdef>int <function>ioctl</function></funcdef>
+ <paramdef>int <parameter>fd</parameter></paramdef>
+ <paramdef>int <parameter>request</parameter></paramdef>
+ <paramdef>struct media_entity_desc *<parameter>argp</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>fd</parameter></term>
+ <listitem>
+ <para>File descriptor returned by
+ <link linkend='media-func-open'><function>open()</function></link>.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>request</parameter></term>
+ <listitem>
+ <para>MEDIA_IOC_ENUM_ENTITIES</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>argp</parameter></term>
+ <listitem>
+ <para></para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Description</title>
+ <para>To query the attributes of an entity, applications set the id field
+ of a &media-entity-desc; structure and call the MEDIA_IOC_ENUM_ENTITIES
+ ioctl with a pointer to this structure. The driver fills the rest of the
+ structure or returns an &EINVAL; when the id is invalid.</para>
+ <para>Entities can be enumerated by or'ing the id with the
+ <constant>MEDIA_ENTITY_ID_FLAG_NEXT</constant> flag. The driver will return
+ information about the entity with the smallest id strictly larger than the
+ requested one ('next entity'), or the &EINVAL; if there is none.</para>
+ <para>Entity IDs can be non-contiguous. Applications must
+ <emphasis>not</emphasis> try to enumerate entities by calling
+ MEDIA_IOC_ENUM_ENTITIES with increasing id's until they get an error.</para>
+ <para>Two or more entities that share a common non-zero
+ <structfield>group_id</structfield> value are considered as logically
+ grouped. Groups are used to report
+ <itemizedlist>
+ <listitem>ALSA, VBI and video nodes that carry the same media
+ stream</listitem>
+ <listitem>lens and flash controllers associated with a sensor</listitem>
+ </itemizedlist>
+ </para>
+
+ <table pgwide="1" frame="none" id="media-entity-desc">
+ <title>struct <structname>media_entity_desc</structname></title>
+ <tgroup cols="5">
+ <colspec colname="c1" />
+ <colspec colname="c2" />
+ <colspec colname="c3" />
+ <colspec colname="c4" />
+ <colspec colname="c5" />
+ <tbody valign="top">
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>id</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Entity id, set by the application. When the id is or'ed with
+ <constant>MEDIA_ENTITY_ID_FLAG_NEXT</constant>, the driver clears
+ the flag and returns the first entity with a larger id.</entry>
+ </row>
+ <row>
+ <entry>char</entry>
+ <entry><structfield>name</structfield>[32]</entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Entity name as an UTF-8 NULL-terminated string.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>type</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Entity type, see <xref linkend="media-entity-type" /> for details.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>revision</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Entity revision in a driver/hardware specific format.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>flags</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Entity flags, see <xref linkend="media-entity-flag" /> for details.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>group_id</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Entity group ID</entry>
+ </row>
+ <row>
+ <entry>__u16</entry>
+ <entry><structfield>pads</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Number of pads</entry>
+ </row>
+ <row>
+ <entry>__u16</entry>
+ <entry><structfield>links</structfield></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>Total number of outbound links. Inbound links are not counted
+ in this field.</entry>
+ </row>
+ <row>
+ <entry>union</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry>struct</entry>
+ <entry><structfield>v4l</structfield></entry>
+ <entry></entry>
+ <entry>Valid for V4L sub-devices and nodes only.</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry></entry>
+ <entry>__u32</entry>
+ <entry><structfield>major</structfield></entry>
+ <entry>V4L device node major number. For V4L sub-devices with no
+ device node, set by the driver to 0.</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry></entry>
+ <entry>__u32</entry>
+ <entry><structfield>minor</structfield></entry>
+ <entry>V4L device node minor number. For V4L sub-devices with no
+ device node, set by the driver to 0.</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry>struct</entry>
+ <entry><structfield>fb</structfield></entry>
+ <entry></entry>
+ <entry>Valid for frame buffer nodes only.</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry></entry>
+ <entry>__u32</entry>
+ <entry><structfield>major</structfield></entry>
+ <entry>Frame buffer device node major number.</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry></entry>
+ <entry>__u32</entry>
+ <entry><structfield>minor</structfield></entry>
+ <entry>Frame buffer device node minor number.</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry>int</entry>
+ <entry><structfield>alsa</structfield></entry>
+ <entry></entry>
+ <entry>ALSA card number</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry>int</entry>
+ <entry><structfield>dvb</structfield></entry>
+ <entry></entry>
+ <entry>DVB card number</entry>
+ </row>
+ <row>
+ <entry></entry>
+ <entry>__u8</entry>
+ <entry><structfield>raw</structfield>[184]</entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table frame="none" pgwide="1" id="media-entity-type">
+ <title>Media entity types</title>
+ <tgroup cols="2">
+ <colspec colname="c1"/>
+ <colspec colname="c2"/>
+ <tbody valign="top">
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_NODE</constant></entry>
+ <entry>Unknown device node</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_NODE_V4L</constant></entry>
+ <entry>V4L video, radio or vbi device node</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_NODE_FB</constant></entry>
+ <entry>Frame buffer device node</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_NODE_ALSA</constant></entry>
+ <entry>ALSA card</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_NODE_DVB</constant></entry>
+ <entry>DVB card</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_SUBDEV</constant></entry>
+ <entry>Unknown V4L sub-device</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_SUBDEV_SENSOR</constant></entry>
+ <entry>Video sensor</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_SUBDEV_FLASH</constant></entry>
+ <entry>Flash controller</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_ENTITY_TYPE_SUBDEV_LENS</constant></entry>
+ <entry>Lens controller</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table frame="none" pgwide="1" id="media-entity-flag">
+ <title>Media entity flags</title>
+ <tgroup cols="2">
+ <colspec colname="c1"/>
+ <colspec colname="c2"/>
+ <tbody valign="top">
+ <row>
+ <entry><constant>MEDIA_ENTITY_FLAG_DEFAULT</constant></entry>
+ <entry>Default entity for its type. Used to discover the default
+ audio, VBI and video devices, the default camera sensor, ...</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+ </refsect1>
+
+ <refsect1>
+ &return-value;
+
+ <variablelist>
+ <varlistentry>
+ <term><errorcode>EINVAL</errorcode></term>
+ <listitem>
+ <para>The &media-entity-desc; <structfield>id</structfield> references
+ a non-existing entity.</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/v4l/media-ioc-enum-links.xml b/Documentation/DocBook/v4l/media-ioc-enum-links.xml
new file mode 100644
index 0000000..ede6d73
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-ioc-enum-links.xml
@@ -0,0 +1,202 @@
+<refentry id="media-ioc-enum-links">
+ <refmeta>
+ <refentrytitle>ioctl MEDIA_IOC_ENUM_LINKS</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>MEDIA_IOC_ENUM_LINKS</refname>
+ <refpurpose>Enumerate all pads and links for a given entity</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcprototype>
+ <funcdef>int <function>ioctl</function></funcdef>
+ <paramdef>int <parameter>fd</parameter></paramdef>
+ <paramdef>int <parameter>request</parameter></paramdef>
+ <paramdef>struct media_links_enum *<parameter>argp</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>fd</parameter></term>
+ <listitem>
+ <para>File descriptor returned by
+ <link linkend='media-func-open'><function>open()</function></link>.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>request</parameter></term>
+ <listitem>
+ <para>MEDIA_IOC_ENUM_LINKS</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>argp</parameter></term>
+ <listitem>
+ <para></para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para>To enumerate pads and/or links for a given entity, applications set
+ the entity field of a &media-links-enum; structure and initialize the
+ &media-pad-desc; and &media-link-desc; structure arrays pointed by the
+ <structfield>pads</structfield> and <structfield>links</structfield> fields.
+ They then call the MEDIA_IOC_ENUM_LINKS ioctl with a pointer to this
+ structure.</para>
+ <para>If the <structfield>pads</structfield> field is not NULL, the driver
+ fills the <structfield>pads</structfield> 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.</para>
+ <para>If the <structfield>links</structfield> field is not NULL, the driver
+ fills the <structfield>links</structfield> 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.</para>
+ <para>Only forward links that originate at one of the entity's source pads
+ are returned during the enumeration process.</para>
+
+ <table pgwide="1" frame="none" id="media-links-enum">
+ <title>struct <structname>media_links_enum</structname></title>
+ <tgroup cols="3">
+ &cs-str;
+ <tbody valign="top">
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>entity</structfield></entry>
+ <entry>Entity id, set by the application.</entry>
+ </row>
+ <row>
+ <entry>struct &media-pad-desc;</entry>
+ <entry>*<structfield>pads</structfield></entry>
+ <entry>Pointer to a pads array allocated by the application. Ignored
+ if NULL.</entry>
+ </row>
+ <row>
+ <entry>struct &media-link-desc;</entry>
+ <entry>*<structfield>links</structfield></entry>
+ <entry>Pointer to a links array allocated by the application. Ignored
+ if NULL.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table pgwide="1" frame="none" id="media-pad-desc">
+ <title>struct <structname>media_pad_desc</structname></title>
+ <tgroup cols="3">
+ &cs-str;
+ <tbody valign="top">
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>entity</structfield></entry>
+ <entry>ID of the entity this pad belongs to.</entry>
+ </row>
+ <row>
+ <entry>__u16</entry>
+ <entry><structfield>index</structfield></entry>
+ <entry>0-based pad index.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>flags</structfield></entry>
+ <entry>Pad flags, see <xref linkend="media-pad-flag" /> for more details.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table frame="none" pgwide="1" id="media-pad-flag">
+ <title>Media pad flags</title>
+ <tgroup cols="2">
+ <colspec colname="c1"/>
+ <colspec colname="c2"/>
+ <tbody valign="top">
+ <row>
+ <entry><constant>MEDIA_PAD_FLAG_INPUT</constant></entry>
+ <entry>Input pad, relative to the entity. Input pads sink data and
+ are targets of links.</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_PAD_FLAG_OUTPUT</constant></entry>
+ <entry>Output pad, relative to the entity. Output pads source data
+ and are origins of links.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table pgwide="1" frame="none" id="media-link-desc">
+ <title>struct <structname>media_links_enum</structname></title>
+ <tgroup cols="3">
+ &cs-str;
+ <tbody valign="top">
+ <row>
+ <entry>struct &media-pad-desc;</entry>
+ <entry><structfield>source</structfield></entry>
+ <entry>Pad at the origin of this link.</entry>
+ </row>
+ <row>
+ <entry>struct &media-pad-desc;</entry>
+ <entry><structfield>sink</structfield></entry>
+ <entry>Pad at the target of this link.</entry>
+ </row>
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>flags</structfield></entry>
+ <entry>Link flags, see <xref linkend="media-link-flag" /> for more details.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table frame="none" pgwide="1" id="media-link-flag">
+ <title>Media link flags</title>
+ <tgroup cols="2">
+ <colspec colname="c1"/>
+ <colspec colname="c2"/>
+ <tbody valign="top">
+ <row>
+ <entry><constant>MEDIA_LINK_FLAG_ACTIVE</constant></entry>
+ <entry>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.</entry>
+ </row>
+ <row>
+ <entry><constant>MEDIA_LINK_FLAG_IMMUTABLE</constant></entry>
+ <entry>The link active state can't be modified at runtime. An
+ immutable link is always active.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+ <para>One and only one of <constant>MEDIA_PAD_FLAG_INPUT</constant> and
+ <constant>MEDIA_PAD_FLAG_OUTPUT</constant> must be set for every pad.</para>
+ </refsect1>
+
+ <refsect1>
+ &return-value;
+
+ <variablelist>
+ <varlistentry>
+ <term><errorcode>EINVAL</errorcode></term>
+ <listitem>
+ <para>The &media-links-enum; <structfield>id</structfield> references
+ a non-existing entity.</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+</refentry>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index d02038a..2e4fe0c 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -61,6 +61,117 @@ static int media_device_get_info(struct media_device *dev,
return copy_to_user(__info, &info, sizeof(*__info));
}

+static struct media_entity *find_entity(struct media_device *mdev, u32 id)
+{
+ struct media_entity *entity;
+ int next = id & MEDIA_ENTITY_ID_FLAG_NEXT;
+
+ id &= ~MEDIA_ENTITY_ID_FLAG_NEXT;
+
+ spin_lock(&mdev->lock);
+
+ media_device_for_each_entity(entity, mdev) {
+ if ((entity->id == id && !next) ||
+ (entity->id > id && next)) {
+ spin_unlock(&mdev->lock);
+ return entity;
+ }
+ }
+
+ spin_unlock(&mdev->lock);
+
+ return NULL;
+}
+
+static long media_device_enum_entities(struct media_device *mdev,
+ struct media_entity_desc __user *uent)
+{
+ struct media_entity *ent;
+ struct media_entity_desc u_ent;
+
+ if (copy_from_user(&u_ent.id, &uent->id, sizeof(u_ent.id)))
+ return -EFAULT;
+
+ ent = find_entity(mdev, u_ent.id);
+
+ if (ent == NULL)
+ return -EINVAL;
+
+ u_ent.id = ent->id;
+ u_ent.name[0] = '\0';
+ if (ent->name)
+ strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
+ u_ent.type = ent->type;
+ u_ent.revision = ent->revision;
+ u_ent.flags = ent->flags;
+ u_ent.group_id = ent->group_id;
+ u_ent.pads = ent->num_pads;
+ u_ent.links = ent->num_links - ent->num_backlinks;
+ u_ent.v4l.major = ent->v4l.major;
+ u_ent.v4l.minor = ent->v4l.minor;
+ if (copy_to_user(uent, &u_ent, sizeof(u_ent)))
+ return -EFAULT;
+ return 0;
+}
+
+static void media_device_kpad_to_upad(const struct media_pad *kpad,
+ struct media_pad_desc *upad)
+{
+ upad->entity = kpad->entity->id;
+ upad->index = kpad->index;
+ upad->flags = kpad->flags;
+}
+
+static long media_device_enum_links(struct media_device *mdev,
+ struct media_links_enum __user *ulinks)
+{
+ struct media_entity *entity;
+ struct media_links_enum links;
+
+ if (copy_from_user(&links, ulinks, sizeof(links)))
+ return -EFAULT;
+
+ entity = find_entity(mdev, links.entity);
+ if (entity == NULL)
+ return -EINVAL;
+
+ if (links.pads) {
+ unsigned int p;
+
+ for (p = 0; p < entity->num_pads; p++) {
+ struct media_pad_desc pad;
+ media_device_kpad_to_upad(&entity->pads[p], &pad);
+ if (copy_to_user(&links.pads[p], &pad, sizeof(pad)))
+ return -EFAULT;
+ }
+ }
+
+ if (links.links) {
+ struct media_link_desc __user *ulink;
+ unsigned int l;
+
+ for (l = 0, ulink = links.links; l < entity->num_links; l++) {
+ struct media_link_desc link;
+
+ /* Ignore backlinks. */
+ if (entity->links[l].source->entity != entity)
+ continue;
+
+ media_device_kpad_to_upad(entity->links[l].source,
+ &link.source);
+ media_device_kpad_to_upad(entity->links[l].sink,
+ &link.sink);
+ link.flags = entity->links[l].flags;
+ if (copy_to_user(ulink, &link, sizeof(*ulink)))
+ return -EFAULT;
+ ulink++;
+ }
+ }
+ if (copy_to_user(ulinks, &links, sizeof(*ulinks)))
+ return -EFAULT;
+ return 0;
+}
+
static long media_device_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
@@ -74,6 +185,18 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
(struct media_device_info __user *)arg);
break;

+ case MEDIA_IOC_ENUM_ENTITIES:
+ ret = media_device_enum_entities(dev,
+ (struct media_entity_desc __user *)arg);
+ break;
+
+ case MEDIA_IOC_ENUM_LINKS:
+ mutex_lock(&dev->graph_mutex);
+ ret = media_device_enum_links(dev,
+ (struct media_links_enum __user *)arg);
+ mutex_unlock(&dev->graph_mutex);
+ break;
+
default:
ret = -ENOIOCTLCMD;
}
diff --git a/include/linux/media.h b/include/linux/media.h
index 8108fe9..2f6a81c 100644
--- a/include/linux/media.h
+++ b/include/linux/media.h
@@ -40,6 +40,87 @@ struct media_device_info {
__u32 reserved[31];
};

+#define MEDIA_ENTITY_ID_FLAG_NEXT (1 << 31)
+
+#define MEDIA_ENTITY_TYPE_SHIFT 16
+#define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
+#define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
+
+#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV (2 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_SUBDEV_SENSOR (MEDIA_ENTITY_TYPE_SUBDEV + 1)
+#define MEDIA_ENTITY_TYPE_SUBDEV_FLASH (MEDIA_ENTITY_TYPE_SUBDEV + 2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_LENS (MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_ENTITY_FLAG_DEFAULT (1 << 0)
+
+struct media_entity_desc {
+ __u32 id;
+ char name[32];
+ __u32 type;
+ __u32 revision;
+ __u32 flags;
+ __u32 group_id;
+ __u16 pads;
+ __u16 links;
+
+ __u32 reserved[4];
+
+ union {
+ /* Node specifications */
+ struct {
+ __u32 major;
+ __u32 minor;
+ } v4l;
+ struct {
+ __u32 major;
+ __u32 minor;
+ } fb;
+ int alsa;
+ int dvb;
+
+ /* Sub-device specifications */
+ /* Nothing needed yet */
+ __u8 raw[184];
+ };
+};
+
+#define MEDIA_PAD_FLAG_INPUT (1 << 0)
+#define MEDIA_PAD_FLAG_OUTPUT (1 << 1)
+
+struct media_pad_desc {
+ __u32 entity; /* entity ID */
+ __u16 index; /* pad index */
+ __u32 flags; /* pad flags */
+ __u32 reserved[2];
+};
+
+#define MEDIA_LINK_FLAG_ACTIVE (1 << 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE (1 << 1)
+
+struct media_link_desc {
+ struct media_pad_desc source;
+ struct media_pad_desc sink;
+ __u32 flags;
+ __u32 reserved[2];
+};
+
+struct media_links_enum {
+ __u32 entity;
+ /* Should have enough room for pads elements */
+ struct media_pad_desc __user *pads;
+ /* Should have enough room for links elements */
+ struct media_link_desc __user *links;
+ __u32 reserved[4];
+};
+
#define MEDIA_IOC_DEVICE_INFO _IOWR('M', 1, struct media_device_info)
+#define MEDIA_IOC_ENUM_ENTITIES _IOWR('M', 2, struct media_entity_desc)
+#define MEDIA_IOC_ENUM_LINKS _IOWR('M', 3, struct media_links_enum)

#endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index b9694b5..42ea2af 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -24,29 +24,7 @@
#define _MEDIA_ENTITY_H

#include <linux/list.h>
-
-#define MEDIA_ENTITY_TYPE_SHIFT 16
-#define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
-#define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
-
-#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
-#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
-#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
-#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
-#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
-
-#define MEDIA_ENTITY_TYPE_SUBDEV (2 << MEDIA_ENTITY_TYPE_SHIFT)
-#define MEDIA_ENTITY_TYPE_SUBDEV_SENSOR (MEDIA_ENTITY_TYPE_SUBDEV + 1)
-#define MEDIA_ENTITY_TYPE_SUBDEV_FLASH (MEDIA_ENTITY_TYPE_SUBDEV + 2)
-#define MEDIA_ENTITY_TYPE_SUBDEV_LENS (MEDIA_ENTITY_TYPE_SUBDEV + 3)
-
-#define MEDIA_ENTITY_FLAG_DEFAULT (1 << 0)
-
-#define MEDIA_LINK_FLAG_ACTIVE (1 << 0)
-#define MEDIA_LINK_FLAG_IMMUTABLE (1 << 1)
-
-#define MEDIA_PAD_FLAG_INPUT (1 << 0)
-#define MEDIA_PAD_FLAG_OUTPUT (1 << 1)
+#include <linux/media.h>

struct media_link {
struct media_pad *source; /* Source pad */
--
1.7.2.2

2010-11-25 02:31:09

[permalink] [raw]

Subject: [RFC/PATCH v6 08/12] media: Links setup

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 <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
Signed-off-by: Sakari Ailus <[email protected]>
---
Documentation/DocBook/media-entities.tmpl | 2 +
Documentation/DocBook/v4l/media-controller.xml | 1 +
Documentation/DocBook/v4l/media-ioc-setup-link.xml | 90 +++++++++
Documentation/media-framework.txt | 49 +++++
drivers/media/media-device.c | 45 +++++
drivers/media/media-entity.c | 208 ++++++++++++++++++++
include/linux/media.h | 1 +
include/media/media-entity.h | 8 +
8 files changed, 404 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/media-ioc-setup-link.xml

diff --git a/Documentation/DocBook/media-entities.tmpl b/Documentation/DocBook/media-entities.tmpl
index 6e7dae4..679c585 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -94,6 +94,7 @@
<!ENTITY MEDIA-IOC-DEVICE-INFO "<link linkend='media-ioc-device-info'><constant>MEDIA_IOC_DEVICE_INFO</constant></link>">
<!ENTITY MEDIA-IOC-ENUM-ENTITIES "<link linkend='media-ioc-enum-entities'><constant>MEDIA_IOC_ENUM_ENTITIES</constant></link>">
<!ENTITY MEDIA-IOC-ENUM-LINKS "<link linkend='media-ioc-enum-links'><constant>MEDIA_IOC_ENUM_LINKS</constant></link>">
+<!ENTITY MEDIA-IOC-SETUP-LINK "<link linkend='media-ioc-setup-link'><constant>MEDIA_IOC_SETUP_LINK</constant></link>">


<!ENTITY v4l2-std-id "<link linkend='v4l2-std-id'>v4l2_std_id</link>">
@@ -342,6 +343,7 @@
<!ENTITY sub-media-ioc-device-info SYSTEM "v4l/media-ioc-device-info.xml">
<!ENTITY sub-media-ioc-enum-entities SYSTEM "v4l/media-ioc-enum-entities.xml">
<!ENTITY sub-media-ioc-enum-links SYSTEM "v4l/media-ioc-enum-links.xml">
+<!ENTITY sub-media-ioc-setup-link SYSTEM "v4l/media-ioc-setup-link.xml">


<!ENTITY close SYSTEM "v4l/func-close.xml">
diff --git a/Documentation/DocBook/v4l/media-controller.xml b/Documentation/DocBook/v4l/media-controller.xml
index 2c4fd2b..2dc25e1 100644
--- a/Documentation/DocBook/v4l/media-controller.xml
+++ b/Documentation/DocBook/v4l/media-controller.xml
@@ -85,4 +85,5 @@
&sub-media-ioc-device-info;
&sub-media-ioc-enum-entities;
&sub-media-ioc-enum-links;
+ &sub-media-ioc-setup-link;
</appendix>
diff --git a/Documentation/DocBook/v4l/media-ioc-setup-link.xml b/Documentation/DocBook/v4l/media-ioc-setup-link.xml
new file mode 100644
index 0000000..84323d0
--- /dev/null
+++ b/Documentation/DocBook/v4l/media-ioc-setup-link.xml
@@ -0,0 +1,90 @@
+<refentry id="media-ioc-setup-link">
+ <refmeta>
+ <refentrytitle>ioctl MEDIA_IOC_SETUP_LINK</refentrytitle>
+ &manvol;
+ </refmeta>
+
+ <refnamediv>
+ <refname>MEDIA_IOC_SETUP_LINK</refname>
+ <refpurpose>Modify the properties of a link</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <funcsynopsis>
+ <funcprototype>
+ <funcdef>int <function>ioctl</function></funcdef>
+ <paramdef>int <parameter>fd</parameter></paramdef>
+ <paramdef>int <parameter>request</parameter></paramdef>
+ <paramdef>struct media_link_desc *<parameter>argp</parameter></paramdef>
+ </funcprototype>
+ </funcsynopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Arguments</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><parameter>fd</parameter></term>
+ <listitem>
+ <para>File descriptor returned by
+ <link linkend='media-func-open'><function>open()</function></link>.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>request</parameter></term>
+ <listitem>
+ <para>MEDIA_IOC_ENUM_LINKS</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><parameter>argp</parameter></term>
+ <listitem>
+ <para></para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para>To change link properties applications fill a &media-link-desc; 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.</para>
+ <para>The only configurable property is the <constant>ACTIVE</constant> link
+ flag to activate/deactivate a link. Links marked with the
+ <constant>IMMUTABLE</constant> link flag can not be activated or
+ deactivated.</para>
+ <para>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 an &EBUSY;.</para>
+ <para>If the specified link can't be found the driver returns with an
+ &EINVAL;.</para>
+ </refsect1>
+
+ <refsect1>
+ &return-value;
+
+ <variablelist>
+ <varlistentry>
+ <term><errorcode>EBUSY</errorcode></term>
+ <listitem>
+ <para>The link properties can't be changed because the link is
+ currently busy. This can be caused, for instance, by an active media
+ stream (audio or video) on the link. The ioctl shouldn't be retried if
+ no other action is performed before to fix the problem.</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
+ <term><errorcode>EINVAL</errorcode></term>
+ <listitem>
+ <para>The &media-link-desc; references a non-existing link, or the
+ link is immutable and an attempt to modify its configuration was made.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect1>
+</refentry>
diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 9fdbc50..e2987b7 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -257,6 +257,16 @@ When the graph traversal is complete the function will return NULL.
Graph traversal can be interrupted at any moment. No cleanup function call is
required and the graph structure can be freed normally.

+Helper functions can be used to find a link between two given pads, or a pad
+connected to another pad through an active link
+
+ media_entity_find_link(struct media_pad *source,
+ struct media_pad *sink);
+
+ media_entity_remote_source(struct media_pad *pad);
+
+Refer to the kerneldoc documentation for more information.
+

Reference counting and power handling
-------------------------------------
@@ -294,3 +304,42 @@ 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.

+
+Links setup
+-----------
+
+Link properties can be modified at runtime by calling
+
+ media_entity_setup_link(struct media_link *link, u32 flags);
+
+The flags argument contains the requested new link flags.
+
+The only configurable property is the ACTIVE link flag to activate/deactivate
+a link. Links marked with the IMMUTABLE link flag can not be activated or
+deactivated.
+
+When a link is activated or deactivated, the media framework calls the
+link_setup operation for the two entities at the source and sink of the link,
+in that order. If the second link_setup call fails, another link_setup call is
+made on the first entity to restore the original link flags.
+
+Entity drivers must implement the link_setup operation if any of their links
+is non-immutable. The operation must either configure the hardware or store
+the configuration information to be applied later.
+
+Link activation must not have any side effect on other links. If an active
+link at a sink pad prevents another link at the same pad from being
+deactivated, the link_setup operation must return -EBUSY and can't implicitly
+deactivate the first active link.
+
+Activating and deactivating a link has effects on entities' reference counts.
+When two sub-graphs are connected, the reference count of each of them is
+incremented by the total reference count of all node entities in the other
+sub-graph. When two sub-graphs are disconnected, the reverse operation is
+performed. In both cases the set_power operations are called accordingly,
+ensuring that the link_setup calls are made with power active on the source
+and sink entities.
+
+In other words, activating or deactivating a link propagates reference count
+changes through the graph, and the final state is identical to what it would
+have been if the link had been active or inactive from the start.
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 2e4fe0c..7c74200 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -172,6 +172,44 @@ static long media_device_enum_links(struct media_device *mdev,
return 0;
}

+static long media_device_setup_link(struct media_device *mdev,
+ struct media_link_desc __user *_ulink)
+{
+ struct media_link *link = NULL;
+ struct media_link_desc ulink;
+ struct media_entity *source;
+ struct media_entity *sink;
+ int ret;
+
+ if (copy_from_user(&ulink, _ulink, sizeof(ulink)))
+ return -EFAULT;
+
+ /* Find the source and sink entities and link.
+ */
+ source = find_entity(mdev, ulink.source.entity);
+ sink = find_entity(mdev, ulink.sink.entity);
+
+ if (source == NULL || sink == NULL)
+ return -EINVAL;
+
+ if (ulink.source.index >= source->num_pads ||
+ ulink.sink.index >= sink->num_pads)
+ return -EINVAL;
+
+ link = media_entity_find_link(&source->pads[ulink.source.index],
+ &sink->pads[ulink.sink.index]);
+ if (link == NULL)
+ return -EINVAL;
+
+ /* Setup the link on both entities. */
+ ret = __media_entity_setup_link(link, ulink.flags);
+
+ if (copy_to_user(_ulink, &ulink, sizeof(ulink)))
+ return -EFAULT;
+
+ return ret;
+}
+
static long media_device_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
@@ -197,6 +235,13 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
mutex_unlock(&dev->graph_mutex);
break;

+ case MEDIA_IOC_SETUP_LINK:
+ mutex_lock(&dev->graph_mutex);
+ ret = media_device_setup_link(dev,
+ (struct media_link_desc __user *)arg);
+ mutex_unlock(&dev->graph_mutex);
+ break;
+
default:
ret = -ENOIOCTLCMD;
}
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index c15f16b..5ebda28 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -200,6 +200,25 @@ EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
* Power state handling
*/

+/*
+ * Return power count of nodes directly or indirectly connected to
+ * a given entity.
+ */
+static int media_entity_count_node(struct media_entity *entity)
+{
+ struct media_entity_graph graph;
+ int use = 0;
+
+ media_entity_graph_walk_start(&graph, entity);
+
+ while ((entity = media_entity_graph_walk_next(&graph))) {
+ if (media_entity_type(entity) == MEDIA_ENTITY_TYPE_NODE)
+ use += entity->use_count;
+ }
+
+ return use;
+}
+
/* Apply use count to an entity. */
static void media_entity_use_apply_one(struct media_entity *entity, int change)
{
@@ -263,6 +282,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.
@@ -406,3 +451,166 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
return 0;
}
EXPORT_SYMBOL_GPL(media_entity_create_link);
+
+static int __media_entity_setup_link_notify(struct media_link *link, u32 flags)
+{
+ const u32 mask = MEDIA_LINK_FLAG_ACTIVE;
+ int ret;
+
+ /* Notify both entities. */
+ ret = media_entity_call(link->source->entity, link_setup,
+ link->source, link->sink, flags);
+ if (ret < 0 && ret != -ENOIOCTLCMD)
+ return ret;
+
+ ret = media_entity_call(link->sink->entity, link_setup,
+ link->sink, link->source, flags);
+ if (ret < 0 && ret != -ENOIOCTLCMD) {
+ media_entity_call(link->source->entity, link_setup,
+ link->source, link->sink, link->flags);
+ return ret;
+ }
+
+ link->flags = (link->flags & ~mask) | (flags & mask);
+ link->reverse->flags = link->flags;
+
+ return 0;
+}
+
+/**
+ * __media_entity_setup_link - Configure a media link
+ * @link: The link being configured
+ * @flags: Link configuration flags
+ *
+ * The bulk of link setup is handled by the two entities connected through the
+ * link. This function notifies both entities of the link configuration change.
+ *
+ * If the link is immutable or if the current and new configuration are
+ * identical, return immediately.
+ *
+ * The user is expected to hold link->source->parent->mutex. If not,
+ * media_entity_setup_link() should be used instead.
+ */
+int __media_entity_setup_link(struct media_link *link, u32 flags)
+{
+ struct media_entity *source, *sink;
+ int ret = -EBUSY;
+
+ if (link == NULL)
+ return -EINVAL;
+
+ if (link->flags & MEDIA_LINK_FLAG_IMMUTABLE)
+ return link->flags == flags ? 0 : -EINVAL;
+
+ if (link->flags == flags)
+ return 0;
+
+ source = __media_entity_get(link->source->entity);
+ if (!source)
+ return ret;
+
+ sink = __media_entity_get(link->sink->entity);
+ if (!sink)
+ goto err___media_entity_get;
+
+ if (flags & MEDIA_LINK_FLAG_ACTIVE) {
+ ret = media_entity_power_connect(source, sink);
+ if (ret < 0)
+ goto err_media_entity_power_connect;
+ }
+
+ ret = __media_entity_setup_link_notify(link, flags);
+ if (ret < 0)
+ goto err___media_entity_setup_link_notify;
+
+ if (!(flags & MEDIA_LINK_FLAG_ACTIVE))
+ media_entity_power_disconnect(source, sink);
+
+ __media_entity_put(sink);
+ __media_entity_put(source);
+
+ return 0;
+
+err___media_entity_setup_link_notify:
+ if (flags & MEDIA_LINK_FLAG_ACTIVE)
+ media_entity_power_disconnect(source, sink);
+err_media_entity_power_connect:
+ __media_entity_put(sink);
+err___media_entity_get:
+ __media_entity_put(source);
+
+ return ret;
+}
+
+int media_entity_setup_link(struct media_link *link, u32 flags)
+{
+ int ret;
+
+ mutex_lock(&link->source->entity->parent->graph_mutex);
+ ret = __media_entity_setup_link(link, flags);
+ mutex_unlock(&link->source->entity->parent->graph_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(media_entity_setup_link);
+
+/**
+ * media_entity_find_link - Find a link between two pads
+ * @source: Source pad
+ * @sink: Sink pad
+ *
+ * Return a pointer to the link between the two entities. If no such link
+ * exists, return NULL.
+ */
+struct media_link *
+media_entity_find_link(struct media_pad *source, struct media_pad *sink)
+{
+ struct media_link *link;
+ unsigned int i;
+
+ for (i = 0; i < source->entity->num_links; ++i) {
+ link = &source->entity->links[i];
+
+ if (link->source->entity == source->entity &&
+ link->source->index == source->index &&
+ link->sink->entity == sink->entity &&
+ link->sink->index == sink->index)
+ return link;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(media_entity_find_link);
+
+/**
+ * media_entity_remote_source - Find the source pad at the remote end of a link
+ * @pad: Sink pad at the local end of the link
+ *
+ * Search for a remote source pad connected to the given sink pad by iterating
+ * over all links originating or terminating at that pad until an active link is
+ * found.
+ *
+ * Return a pointer to the pad at the remote end of the first found active link,
+ * or NULL if no active link has been found.
+ */
+struct media_pad *media_entity_remote_source(struct media_pad *pad)
+{
+ unsigned int i;
+
+ for (i = 0; i < pad->entity->num_links; i++) {
+ struct media_link *link = &pad->entity->links[i];
+
+ if (!(link->flags & MEDIA_LINK_FLAG_ACTIVE))
+ continue;
+
+ if (link->source == pad)
+ return link->sink;
+
+ if (link->sink == pad)
+ return link->source;
+ }
+
+ return NULL;
+
+}
+EXPORT_SYMBOL_GPL(media_entity_remote_source);
diff --git a/include/linux/media.h b/include/linux/media.h
index 2f6a81c..1df5b6e 100644
--- a/include/linux/media.h
+++ b/include/linux/media.h
@@ -122,5 +122,6 @@ struct media_links_enum {
#define MEDIA_IOC_DEVICE_INFO _IOWR('M', 1, struct media_device_info)
#define MEDIA_IOC_ENUM_ENTITIES _IOWR('M', 2, struct media_entity_desc)
#define MEDIA_IOC_ENUM_LINKS _IOWR('M', 3, struct media_links_enum)
+#define MEDIA_IOC_SETUP_LINK _IOWR('M', 4, struct media_link_desc)

#endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 42ea2af..11bef651 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -40,6 +40,9 @@ struct media_pad {
};

struct media_entity_operations {
+ int (*link_setup)(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32 flags);
int (*set_power)(struct media_entity *entity, int power);
};

@@ -110,6 +113,11 @@ int media_entity_init(struct media_entity *entity, u16 num_pads,
void media_entity_cleanup(struct media_entity *entity);
int media_entity_create_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags);
+int __media_entity_setup_link(struct media_link *link, u32 flags);
+int media_entity_setup_link(struct media_link *link, u32 flags);
+struct media_link *media_entity_find_link(struct media_pad *source,
+ struct media_pad *sink);
+struct media_pad *media_entity_remote_source(struct media_pad *pad);

struct media_entity *media_entity_get(struct media_entity *entity);
void media_entity_put(struct media_entity *entity);
--
1.7.2.2

2010-11-25 02:31:16

[permalink] [raw]

Subject: [RFC/PATCH v6 09/12] media: Entity locking and pipeline management

Link states must not be modified while streaming is in progress on a
graph they belong or connect to. The entity locking API helps drivers
enforcing that requirement.

When starting streaming on a graph, drivers lock all entities in the
graph with a call to media_entity_graph_lock(). Similarly, when stopping
the stream, they unlock the entities with a call to
media_entity_graph_unlock().

The media_entity_graph_lock() function takes a pointer to a media
pipeline and stores it in every entity in the graph. Drivers should
embed the media_pipeline structure in higher-level pipeline structures
and can then access the pipeline through the media_entity structure.

Link configuration will fail with -EBUSY if either end of the link is a
locked entity.

Signed-off-by: Laurent Pinchart <[email protected]>
---
Documentation/media-framework.txt | 35 ++++++++++++++++++++
drivers/media/media-entity.c | 64 +++++++++++++++++++++++++++++++++++++
include/media/media-entity.h | 10 ++++++
3 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index e2987b7..3131e1e 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -343,3 +343,38 @@ 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.
+
+
+Entity locking and pipelines
+----------------------------
+
+When starting streaming, drivers must lock all entities in the graph to
+prevent link states from being modified during streaming by calling
+
+ media_entity_graph_lock(struct media_entity *entity,
+ struct media_pipeline *pipe);
+
+The function will lock all entities connected to the given entity through
+active links, either directly or indirectly.
+
+The media_pipeline instance pointed to by the pipe argument will be stored in
+every entity in the graph. Drivers should embed the media_pipeline structure
+in higher-level pipeline structures and can then access the pipeline through
+the media_entity pipe field.
+
+Calls to media_entity_graph_lock() can be nested. The pipeline pointer must be
+identical for all nested calls to the function.
+
+When stopping the stream, drivers must unlock the entities with
+
+ media_entity_graph_unlock(struct media_entity *entity);
+
+If multiple calls to media_entity_graph_lock() have been made the same number
+of media_entity_graph_unlock() calls are required to unlock the graph. The
+media_entity pipe field is reset to NULL on the last nested unlock call.
+
+Link configuration will fail with -EBUSY if either end of the link is a
+locked entity. If other operations need to be disallowed on locked entities
+(such as changing entities configuration parameters) drivers can explictly
+check the media_entity lock_count field to find out if an entity is locked.
+This operation must be done with the media_device graph_mutex held.
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 5ebda28..3e2670e 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -196,6 +196,67 @@ media_entity_graph_walk_next(struct media_entity_graph *graph)
}
EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);

+/**
+ * media_entity_graph_lock - Lock all entities in a graph
+ * @entity: Starting entity
+ * @pipe: Media pipeline to be assigned to all entities in the graph.
+ *
+ * Lock all entities connected to a given entity through active links, either
+ * directly or indirectly. The given pipeline is assigned to every entity in
+ * the graph and stored in the media_entity pipe field.
+ *
+ * Calls to this function can be nested, in which case the same number of
+ * media_entity_graph_unlock() calls will be required to unlock the graph. The
+ * pipeline pointer must be identical for all nested calls to
+ * media_entity_graph_lock().
+ */
+void media_entity_graph_lock(struct media_entity *entity,
+ struct media_pipeline *pipe)
+{
+ struct media_device *mdev = entity->parent;
+ struct media_entity_graph graph;
+
+ mutex_lock(&mdev->graph_mutex);
+
+ media_entity_graph_walk_start(&graph, entity);
+
+ while ((entity = media_entity_graph_walk_next(&graph))) {
+ entity->lock_count++;
+ WARN_ON(entity->pipe && entity->pipe != pipe);
+ entity->pipe = pipe;
+ }
+
+ mutex_unlock(&mdev->graph_mutex);
+}
+EXPORT_SYMBOL_GPL(media_entity_graph_lock);
+
+/**
+ * media_entity_graph_unlock - Unlock all entities in a graph
+ * @entity: Starting entity
+ *
+ * Unlock all entities connected to a given entity through active links, either
+ * directly or indirectly. The media_entity pipe field is reset to NULL on the
+ * last nested unlock call.
+ */
+void media_entity_graph_unlock(struct media_entity *entity)
+{
+ struct media_device *mdev = entity->parent;
+ struct media_entity_graph graph;
+
+ mutex_lock(&mdev->graph_mutex);
+
+ media_entity_graph_walk_start(&graph, entity);
+
+ while ((entity = media_entity_graph_walk_next(&graph))) {
+ entity->lock_count--;
+ if (entity->lock_count == 0)
+ entity->pipe = NULL;
+ }
+
+ mutex_unlock(&mdev->graph_mutex);
+}
+EXPORT_SYMBOL_GPL(media_entity_graph_unlock);
+
/* -----------------------------------------------------------------------------
* Power state handling
*/
@@ -505,6 +566,9 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
if (link->flags == flags)
return 0;

+ if (link->source->entity->lock_count || link->sink->entity->lock_count)
+ return -EBUSY;
+
source = __media_entity_get(link->source->entity);
if (!source)
return ret;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 11bef651..74030f2 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -26,6 +26,9 @@
#include <linux/list.h>
#include <linux/media.h>

+struct media_pipeline {
+};
+
struct media_link {
struct media_pad *source; /* Source pad */
struct media_pad *sink; /* Sink pad */
@@ -68,8 +71,11 @@ struct media_entity {

const struct media_entity_operations *ops; /* Entity operations */

+ int lock_count; /* Lock count for the entity. */
int use_count; /* Use count for the entity. */

+ struct media_pipeline *pipe; /* Pipeline this entity belongs to. */
+
union {
/* Node specifications */
struct {
@@ -111,6 +117,7 @@ struct media_entity_graph {
int media_entity_init(struct media_entity *entity, u16 num_pads,
struct media_pad *pads, u16 extra_links);
void media_entity_cleanup(struct media_entity *entity);
+
int media_entity_create_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags);
int __media_entity_setup_link(struct media_link *link, u32 flags);
@@ -126,6 +133,9 @@ 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);
+void media_entity_graph_lock(struct media_entity *entity,
+ struct media_pipeline *pipe);
+void media_entity_graph_unlock(struct media_entity *entity);

#define media_entity_call(entity, operation, args...) \
(((entity)->ops && (entity)->ops->operation) ? \
--
1.7.2.2

2010-11-25 02:31:57

[permalink] [raw]

Subject: [RFC/PATCH v6 10/12] v4l: Add a media_device pointer to the v4l2_device structure

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 <[email protected]>
---
Documentation/video4linux/v4l2-framework.txt | 17 ++++++++++++-----
drivers/media/video/v4l2-device.c | 13 +++++++------
include/media/v4l2-device.h | 4 ++++
3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 4db1def..aeb2a22 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 318e911..8447466 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -46,9 +46,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);
@@ -71,10 +70,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 6648036..b649c74 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,
@@ -39,6 +40,9 @@ 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;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_device *mdev;
+#endif
/* 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.2.2

2010-11-25 02:31:58

[permalink] [raw]

Subject: [RFC/PATCH v6 11/12] v4l: Make video_device inherit from media_entity

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 <[email protected]>
Signed-off-by: Sakari Ailus <[email protected]>
---
Documentation/video4linux/v4l2-framework.txt | 38 +++++++++++++++++++--
drivers/media/video/v4l2-dev.c | 47 +++++++++++++++++++++++---
include/media/v4l2-dev.h | 7 ++++
3 files changed, 84 insertions(+), 8 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index aeb2a22..f231bc20 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
@@ -532,6 +539,21 @@ If you use v4l2_ioctl_ops, then you should set either .unlocked_ioctl or
The v4l2_file_operations struct is a subset of file_operations. The main
difference is that the inode argument is omitted since it is never used.

+If integration with the media framework is needed, you must initialize the
+media_entity struct embedded in the video_device struct (entity field) by
+calling media_entity_init():
+
+ struct media_pad *pad = &my_vdev->pad;
+ int err;
+
+ err = media_entity_init(&vdev->entity, 1, pad, 0);
+
+The pads array must have been previously initialized. There is no need to
+manually set the struct media_entity type and name fields.
+
+A reference to the entity will be automatically acquired/released when the
+video device is opened/closed.
+
v4l2_file_operations and locking
--------------------------------

@@ -561,6 +583,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:

@@ -636,6 +661,13 @@ release, of course) will return an error as well.
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 035db52..511e1ee 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -278,6 +278,9 @@ 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;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_entity *entity = NULL;
+#endif
int ret = 0;

/* Check if the video device is available */
@@ -291,6 +294,16 @@ static int v4l2_open(struct inode *inode, struct file *filp)
/* and increase the device refcount */
video_get(vdev);
mutex_unlock(&videodev_lock);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
+ entity = media_entity_get(&vdev->entity);
+ if (!entity) {
+ ret = -EBUSY;
+ video_put(vdev);
+ return ret;
+ }
+ }
+#endif
if (vdev->fops->open) {
if (vdev->lock)
mutex_lock(vdev->lock);
@@ -303,8 +316,13 @@ static int v4l2_open(struct inode *inode, struct file *filp)
}

/* decrease the refcount in case of an error */
- if (ret)
+ if (ret) {
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
+ media_entity_put(entity);
+#endif
video_put(vdev);
+ }
return ret;
}

@@ -321,7 +339,10 @@ static int v4l2_release(struct inode *inode, struct file *filp)
if (vdev->lock)
mutex_unlock(vdev->lock);
}
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
+ media_entity_put(&vdev->entity);
+#endif
/* decrease the refcount unconditionally since the release()
return value is ignored. */
video_put(vdev);
@@ -558,12 +579,25 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
if (nr != -1 && nr != vdev->num && warn_if_nr_in_use)
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. */
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ /* Part 5: Register the entity. */
+ if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
+ vdev->entity.type = MEDIA_ENTITY_TYPE_NODE_V4L;
+ vdev->entity.name = vdev->name;
+ vdev->entity.v4l.major = VIDEO_MAJOR;
+ vdev->entity.v4l.minor = vdev->minor;
+ ret = media_device_register_entity(vdev->v4l2_dev->mdev,
+ &vdev->entity);
+ if (ret < 0)
+ printk(KERN_ERR "error\n"); /* TODO */
+ }
+#endif
+ /* 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:
@@ -590,7 +624,10 @@ void video_unregister_device(struct video_device *vdev)
/* Check if vdev was ever registered at all */
if (!vdev || !video_is_registered(vdev))
return;
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
+ media_device_unregister_entity(&vdev->entity);
+#endif
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 4fe6831..51b2c51 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
@@ -55,6 +57,9 @@ struct v4l2_file_operations {

struct video_device
{
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_entity entity;
+#endif
/* device ops */
const struct v4l2_file_operations *fops;

@@ -100,6 +105,8 @@ struct video_device
struct mutex *lock;
};

+#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.2.2

2010-11-25 02:32:38

[permalink] [raw]

Subject: [RFC/PATCH v6 12/12] v4l: Make v4l2_subdev inherit from media_entity

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 <[email protected]>
Signed-off-by: Sakari Ailus <[email protected]>
---
Documentation/video4linux/v4l2-framework.txt | 23 ++++++++++++++
drivers/media/video/v4l2-device.c | 39 ++++++++++++++++++++----
drivers/media/video/v4l2-subdev.c | 41 ++++++++++++++++++++++++-
include/media/v4l2-subdev.h | 10 ++++++
4 files changed, 104 insertions(+), 9 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index f231bc20..d0fb880 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -268,6 +268,26 @@ A sub-device driver initializes the v4l2_subdev struct using:
Afterwards you need to initialize subdev->name with a unique name and set the
module owner. This is done for you if you use the i2c helper functions.

+If integration with the media framework is needed, you must initialize the
+media_entity struct embedded in the v4l2_subdev struct (entity field) by
+calling media_entity_init():
+
+ struct media_pad *pads = &my_sd->pads;
+ int err;
+
+ err = media_entity_init(&sd->entity, npads, pads, 0);
+
+The pads array must have been previously initialized. There is no need to
+manually set the struct media_entity type and name fields, but the revision
+field must be initialized if needed.
+
+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:

@@ -277,6 +297,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 8447466..7ac4d0f 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -115,8 +115,11 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)
EXPORT_SYMBOL_GPL(v4l2_device_unregister);

int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
- struct v4l2_subdev *sd)
+ struct v4l2_subdev *sd)
{
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_entity *entity = &sd->entity;
+#endif
struct video_device *vdev;
int err;

@@ -134,7 +137,16 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler);
if (err)
return err;
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ /* Register the entity. */
+ if (v4l2_dev->mdev) {
+ err = media_device_register_entity(v4l2_dev->mdev, entity);
+ if (err < 0) {
+ module_put(sd->owner);
+ return err;
+ }
+ }
+#endif
sd->v4l2_dev = v4l2_dev;
spin_lock(&v4l2_dev->lock);
list_add_tail(&sd->list, &v4l2_dev->subdevs);
@@ -149,26 +161,39 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) {
err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
sd->owner);
- if (err < 0)
+ if (err < 0) {
v4l2_device_unregister_subdev(sd);
+ return err;
+ }
}
-
- return err;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ entity->v4l.major = VIDEO_MAJOR;
+ entity->v4l.minor = vdev->minor;
+#endif
+ 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 defined(CONFIG_MEDIA_CONTROLLER)
+ if (v4l2_dev->mdev)
+ media_device_unregister_entity(&sd->entity);
+#endif
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 002281c..3f09e23 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -34,7 +34,10 @@ 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;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_entity *entity;
+#endif
+ struct v4l2_fh *vfh = NULL;
int ret;

if (!sd->initialized)
@@ -60,11 +63,20 @@ static int subdev_open(struct file *file)
v4l2_fh_add(vfh);
file->private_data = vfh;
}
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (sd->v4l2_dev->mdev) {
+ entity = media_entity_get(&sd->entity);
+ if (!entity) {
+ ret = -EBUSY;
+ goto err;
+ }
+ }
+#endif
return 0;

err:
if (vfh != NULL) {
+ v4l2_fh_del(vfh);
v4l2_fh_exit(vfh);
kfree(vfh);
}
@@ -74,8 +86,16 @@ err:

static int subdev_close(struct file *file)
{
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct video_device *vdev = video_devdata(file);
+ struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+#endif
struct v4l2_fh *vfh = file->private_data;

+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (sd->v4l2_dev->mdev)
+ media_entity_put(&sd->entity);
+#endif
if (vfh != NULL) {
v4l2_fh_del(vfh);
v4l2_fh_exit(vfh);
@@ -175,5 +195,22 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
sd->dev_priv = NULL;
sd->host_priv = NULL;
sd->initialized = 1;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ sd->entity.name = sd->name;
+ sd->entity.type = MEDIA_ENTITY_TYPE_SUBDEV;
+#endif
}
EXPORT_SYMBOL(v4l2_subdev_init);
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+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);
+#endif
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 68cbe48..7d55b0c 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>
@@ -437,6 +438,9 @@ struct v4l2_subdev_ops {
stand-alone or embedded in a larger struct.
*/
struct v4l2_subdev {
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ struct media_entity entity;
+#endif
struct list_head list;
struct module *owner;
u32 flags;
@@ -458,6 +462,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)

@@ -486,6 +492,10 @@ static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
void v4l2_subdev_init(struct v4l2_subdev *sd,
const struct v4l2_subdev_ops *ops);

+#if defined(CONFIG_MEDIA_CONTROLLER)
+int v4l2_subdev_set_power(struct media_entity *entity, int power);
+#endif
+
/* Call an ops of a v4l2_subdev, doing the right checks against
NULL pointers.

--
1.7.2.2

2010-11-25 09:32:45

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 02/12] media: Media device

Laurent Pinchart wrote:
> +struct media_device {
> ...
> + u8 model[32];
> + u8 serial[40];
> + u8 bus_info[32];

All drivers and userspace applications have to treat this as char[], so
why u8[]?

Regards,
Clemens

2010-11-25 09:35:58

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

Laurent Pinchart wrote:
> 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.

In the descriptors of USB Audio and HDAudio devices, the links are
stored only in the sink. But AFAICS this doesn't matter in the media
driver API.

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

So the intended userspace API for controlling routing is to change the
ACTIVE flag of links?

In USB and HD audio devices, all links are immutable, and the routing
is controlled by 'selector' entities that activate exactly one of their
input pads. In userspace, this entity shows up as a mixer control.
I guess it would be possible to map the ACTIVE flag onto these controls.

Alternatively, entities can have 'mute' mixer controls associated with
their pads. In this case, multiple unmuted inputs would be mixed
together.

> +#define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
> +#define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
> +
> +#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
> ...
> +#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)

ALSA has PCM and MIDI devices, and several types of mixer controls.
(It also has hardware dependent and timer devices, but I don't think
these would need topology information.) So we need at least these:
MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL

Furthermore, topology information is also needed for entities not
associated with a mixer control, such as microphones, speakers, jacks/
connectors, and effect units. These entities are defined in the USB and
HD audio specifications, but are not yet handled by ALSA.

> +struct media_entity {
> ...
> + 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 */
> + };

ALSA devices are not addressed by their device node but with card/device/
subdevice numbers; mixer controls have numeric IDs, unique per card:

struct {
int card;
int device;
int subdevice;
} alsa_device;
struct {
int card;
int numid;
} alsa_control;

Regards,
Clemens

2010-11-25 11:38:29

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 11/12] v4l: Make video_device inherit from media_entity

On Thursday, November 25, 2010 03:28:18 Laurent Pinchart wrote:
> 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 <[email protected]>
> Signed-off-by: Sakari Ailus <[email protected]>
> ---
> Documentation/video4linux/v4l2-framework.txt | 38 +++++++++++++++++++--
> drivers/media/video/v4l2-dev.c | 47 +++++++++++++++++++++++---
> include/media/v4l2-dev.h | 7 ++++
> 3 files changed, 84 insertions(+), 8 deletions(-)
>

<snip>

> diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> index 035db52..511e1ee 100644
> --- a/drivers/media/video/v4l2-dev.c
> +++ b/drivers/media/video/v4l2-dev.c
> @@ -278,6 +278,9 @@ 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;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + struct media_entity *entity = NULL;
> +#endif
> int ret = 0;
>
> /* Check if the video device is available */
> @@ -291,6 +294,16 @@ static int v4l2_open(struct inode *inode, struct file *filp)
> /* and increase the device refcount */
> video_get(vdev);
> mutex_unlock(&videodev_lock);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
> + entity = media_entity_get(&vdev->entity);
> + if (!entity) {
> + ret = -EBUSY;
> + video_put(vdev);
> + return ret;
> + }
> + }
> +#endif
> if (vdev->fops->open) {
> if (vdev->lock)
> mutex_lock(vdev->lock);
> @@ -303,8 +316,13 @@ static int v4l2_open(struct inode *inode, struct file *filp)
> }
>
> /* decrease the refcount in case of an error */
> - if (ret)
> + if (ret) {
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
> + media_entity_put(entity);
> +#endif
> video_put(vdev);
> + }
> return ret;
> }
>
> @@ -321,7 +339,10 @@ static int v4l2_release(struct inode *inode, struct file *filp)
> if (vdev->lock)
> mutex_unlock(vdev->lock);
> }
> -
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
> + media_entity_put(&vdev->entity);
> +#endif
> /* decrease the refcount unconditionally since the release()
> return value is ignored. */
> video_put(vdev);
> @@ -558,12 +579,25 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
> if (nr != -1 && nr != vdev->num && warn_if_nr_in_use)
> 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. */
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + /* Part 5: Register the entity. */
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
> + vdev->entity.type = MEDIA_ENTITY_TYPE_NODE_V4L;
> + vdev->entity.name = vdev->name;
> + vdev->entity.v4l.major = VIDEO_MAJOR;
> + vdev->entity.v4l.minor = vdev->minor;
> + ret = media_device_register_entity(vdev->v4l2_dev->mdev,
> + &vdev->entity);
> + if (ret < 0)
> + printk(KERN_ERR "error\n"); /* TODO */

Was this forgotten, or will this be fixed in the next version? It looks
out-of-place...

> + }
> +#endif

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by Cisco

2010-11-25 13:36:44

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Thu, Nov 25, 2010 at 03:28:10AM +0100, Laurent Pinchart wrote:

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

Is this supposed to reflect the current state (if the link is carrying
data right now) or if it's possible for the link to carry data?

> +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 revision; /* Entity revision, driver specific */
> + unsigned long flags; /* Entity flags (MEDIA_ENTITY_FLAG_*) */
> + u32 group_id; /* Entity group ID */
> +
> + u16 num_pads; /* Number of input and output pads */
> + u16 num_links; /* Number of existing links, both active
> + * and inactive */
> + u16 num_backlinks; /* Number of backlinks */
> + u16 max_links; /* Maximum number of links */
> +
> + struct media_pad *pads; /* Pads array (num_pads elements) */
> + struct media_link *links; /* Links array (max_links elements)*/

Hrm. This is getting kind of large, especially considering the volume
of data we're holding per node and link already in ASoC. On the other
hand we may over time be able to refactor some of the existing stuff
(especially the link management) to use this structure.

2010-11-25 13:41:29

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

On Thu, Nov 25, 2010 at 10:38:05AM +0100, Clemens Ladisch wrote:

> In USB and HD audio devices, all links are immutable, and the routing
> is controlled by 'selector' entities that activate exactly one of their
> input pads. In userspace, this entity shows up as a mixer control.
> I guess it would be possible to map the ACTIVE flag onto these controls.

Ditto for ASoC, mostly.

> Alternatively, entities can have 'mute' mixer controls associated with
> their pads. In this case, multiple unmuted inputs would be mixed
> together.

> ALSA has PCM and MIDI devices, and several types of mixer controls.
> (It also has hardware dependent and timer devices, but I don't think
> these would need topology information.) So we need at least these:
> MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
> MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
> MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL

> Furthermore, topology information is also needed for entities not
> associated with a mixer control, such as microphones, speakers, jacks/
> connectors, and effect units. These entities are defined in the USB and
> HD audio specifications, but are not yet handled by ALSA.

All this and more in the embedded case - digital audio link nodes and DSP
I/O nodes (could possibly do those as digital audio ones) spring to
mind. Also bear in mind that embedded devices can get *very* large - a
mobile phone audio system can have of the order of 100 nodes in the
graph.

> ALSA devices are not addressed by their device node but with card/device/
> subdevice numbers; mixer controls have numeric IDs, unique per card:
>
> struct {
> int card;
> int device;
> int subdevice;
> } alsa_device;
> struct {
> int card;
> int numid;
> } alsa_control;

For the embedded stuff we also have a bunch of stuff in the graph which
may not be visible to userspace at all at present and would just have a
string based identifier.

2010-11-25 13:49:02

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 05/12] media: Reference count and power handling

On Thu, Nov 25, 2010 at 03:28:12AM +0100, Laurent Pinchart wrote:

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

ASoC has its own power management stuff which doesn't *quite* map onto
this one as-is. The power determination stuff is essentially identical
(and this is actually nicer) but we have a separate postprocessing stage
that actually applies the changes in a sequence which minimises audible
issues caused by doing things in a bad order (eg, power down a PGA
before you turn off inputs). This is noddy enough to implement, though
- we just need a pre and post run notifications to set up and implement
the changes I think.

BTW, I notice you've not CCed the ALSA list, Liam, Takashi or Jaroslav
on any of this - might be good.

2010-11-25 13:53:44

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 09/12] media: Entity locking and pipeline management

On Thu, Nov 25, 2010 at 03:28:16AM +0100, Laurent Pinchart wrote:
> Link states must not be modified while streaming is in progress on a
> graph they belong or connect to. The entity locking API helps drivers
> enforcing that requirement.

This is not desirable for embedded audio - for example, it is very
common to wish to switch from headphone to speaker and back mid
playback, or to move between headset, speakerphone and handset modes on
a phone. This is normally implemented by reconfiguring the output paths
at runtime without interrupting applications.

2010-11-25 14:28:08

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 00/12] Media controller (core and V4L2)

On Thu, Nov 25, 2010 at 03:28:07AM +0100, Laurent Pinchart wrote:

> I want to emphasize that the media controller API does *not* replace the V4L,
> DVB or ALSA APIs. It complements them.

Overall this looks relatively good and should be mappable onto the
embedded audio stack. I'd need to sit down and actually do that to
go into too much specific detail but the only real issue I see is
working out how the control aspects of this interact with the control in
ALSA. The simplest thing would be to have the userspace API be read
only for ALSA devices, I guess.

One potential issue

2010-11-25 14:37:40

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 11/12] v4l: Make video_device inherit from media_entity

Hi Hans,

Thanks for the review.

On Thursday 25 November 2010 12:38:15 Hans Verkuil wrote:
> On Thursday, November 25, 2010 03:28:18 Laurent Pinchart wrote:
> > 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 <[email protected]>
> > Signed-off-by: Sakari Ailus <[email protected]>
> > ---
> >
> > Documentation/video4linux/v4l2-framework.txt | 38
> > +++++++++++++++++++-- drivers/media/video/v4l2-dev.c |
> > 47 +++++++++++++++++++++++--- include/media/v4l2-dev.h
> > | 7 ++++
> > 3 files changed, 84 insertions(+), 8 deletions(-)
>
> <snip>
>
> > diff --git a/drivers/media/video/v4l2-dev.c
> > b/drivers/media/video/v4l2-dev.c index 035db52..511e1ee 100644
> > --- a/drivers/media/video/v4l2-dev.c
> > +++ b/drivers/media/video/v4l2-dev.c

[snip]

> > @@ -558,12 +579,25 @@ int __video_register_device(struct video_device
> > *vdev, int type, int nr,
> >
> > if (nr != -1 && nr != vdev->num && warn_if_nr_in_use)
> >
> > 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. */
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > + /* Part 5: Register the entity. */
> > + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
> > + vdev->entity.type = MEDIA_ENTITY_TYPE_NODE_V4L;
> > + vdev->entity.name = vdev->name;
> > + vdev->entity.v4l.major = VIDEO_MAJOR;
> > + vdev->entity.v4l.minor = vdev->minor;
> > + ret = media_device_register_entity(vdev->v4l2_dev->mdev,
> > + &vdev->entity);
> > + if (ret < 0)
> > + printk(KERN_ERR "error\n"); /* TODO */
>
> Was this forgotten, or will this be fixed in the next version? It looks
> out-of-place...

OOPS. I totally forgot about that one. I'll fix it for the next version.

--
Regards,

Laurent Pinchart

2010-11-25 14:42:01

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 02/12] media: Media device

Hi Clemens,

Thanks for the review.

On Thursday 25 November 2010 10:33:02 Clemens Ladisch wrote:
> Laurent Pinchart wrote:
> > +struct media_device {
> > ...
> > + u8 model[32];
> > + u8 serial[40];
> > + u8 bus_info[32];
>
> All drivers and userspace applications have to treat this as char[], so
> why u8[]?

Good question. I've copied the V4L2 practice of using u8 (or __u8) for fixed-
length strings in structures. I can't think of any reason for that.

I will replace u8 with char unless someone comes up with a good reason to keep
u8.

--
Regards,

Laurent Pinchart

2010-11-25 15:21:28

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

Hi Clemens,

Thanks a lot for the review.

On Thursday 25 November 2010 10:38:05 Clemens Ladisch wrote:
> Laurent Pinchart wrote:
> > 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.
>
> In the descriptors of USB Audio and HDAudio devices, the links are
> stored only in the sink. But AFAICS this doesn't matter in the media
> driver API.

Same for USB Video (UVC). The reason for that is that UAC and UVC have only 1-
to-many connections (same for the media controller by the way), and it's
easier to store that information in fixed-size structures if you store it in
the sink entities.

On the kernel side, the media controller stores links in both source and sink
entities for internal reasons. Links stored in the sink entity are called
backlinks.

On the userspace side applications will only see "forward" links, stored in
source entities. Applications will of course be free to store the links
whereever they want in their internal data structures.

As you mentioned, I think it doesn't matter much anyway.

> > +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.
>
> So the intended userspace API for controlling routing is to change the
> ACTIVE flag of links?

That's correct.

> In USB and HD audio devices, all links are immutable, and the routing
> is controlled by 'selector' entities that activate exactly one of their
> input pads. In userspace, this entity shows up as a mixer control.
> I guess it would be possible to map the ACTIVE flag onto these controls.

Same for UVC once again. It would be quite easy to map selector units to link
activation, but it doesn't even have to be done. We could keep the selector
units in the graph with all links immutable, and use the existing APIs to
control the selector units.

The alternative would have been to add selector units to the media controller.
I've thought about it, but we have embedded hardware (at least on the video
side) that have configurable links without selector units. The media
controller would have had to create fake selector units.

Between creating fake selector and translating link activation to selector
unit commands, I've decided to go for the second solution, as this will make
the graph simpler. Once again we can decide to keep explicit selector units in
the graph for UVC, UAC and HD audio devices. That's one I've done in a test
implementation of the media controller API in the uvcvideo driver.

> Alternatively, entities can have 'mute' mixer controls associated with
> their pads. In this case, multiple unmuted inputs would be mixed
> together.

I don't see any problem keeping those.

> > +#define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
> > +#define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
> > +
> > +#define MEDIA_ENTITY_TYPE_NODE (1 <<
> > MEDIA_ENTITY_TYPE_SHIFT) ...
> > +#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE +
> > 3)
>
> ALSA has PCM and MIDI devices, and several types of mixer controls.
> (It also has hardware dependent and timer devices, but I don't think
> these would need topology information.) So we need at least these:
> MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
> MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
> MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL

I agree about PCM and MIDI, but I'm not sure about controls. If controls are
part of an entity, the entity will be reported through the media controller
API. If information about that entity can be queried through existing APIs
(ALSA, V4L, ...) those APIs should be used. For instance, on the V4L side,
V4L2 sub-devices are mapped to entities and also have a device node which can
be used to enumerate the controls supported by the subdev. The media
controller only reports the entity -> major:minor mapping to let applications
find the device and query it directly.

I think we will need a new ioctl in the media controller API to report
advanced information about an entity. This could be used to report controls
implemented by an ALSA element if ALSA doesn't provide a way to do that
directly. Another use case, which make me think that such an ioctl would be
needed, is to report UVC extension units type (16-byte GUID) to userspace.

> Furthermore, topology information is also needed for entities not
> associated with a mixer control, such as microphones, speakers, jacks/
> connectors, and effect units. These entities are defined in the USB and
> HD audio specifications, but are not yet handled by ALSA.

Agreed, we will need to add new entity types and subtypes for those. The
reason they're not part of this submission is that I wanted real use cases
instead of coming up with a made-up list of entities I think would be useful.

> > +struct media_entity {
> > ...
> > + 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 */
> > + };
>
> ALSA devices are not addressed by their device node but with card/device/
> subdevice numbers; mixer controls have numeric IDs, unique per card:
>
> struct {
> int card;
> int device;
> int subdevice;
> } alsa_device;

I will use this instead of 'int alsa'. Thanks a lot for the useful feedback.

> struct {
> int card;
> int numid;
> } alsa_control;

Regarding controls, let's first see how they should be reported by userspace,
as explained above.

--
Regards,

Laurent Pinchart

2010-11-25 15:28:31

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

On Thu, Nov 25, 2010 at 04:21:38PM +0100, Laurent Pinchart wrote:
> On Thursday 25 November 2010 10:38:05 Clemens Ladisch wrote:

> > ALSA has PCM and MIDI devices, and several types of mixer controls.
> > (It also has hardware dependent and timer devices, but I don't think
> > these would need topology information.) So we need at least these:
> > MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
> > MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
> > MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL

> I agree about PCM and MIDI, but I'm not sure about controls. If controls are
> part of an entity, the entity will be reported through the media controller
> API. If information about that entity can be queried through existing APIs
> (ALSA, V4L, ...) those APIs should be used. For instance, on the V4L side,
> V4L2 sub-devices are mapped to entities and also have a device node which can
> be used to enumerate the controls supported by the subdev. The media
> controller only reports the entity -> major:minor mapping to let applications
> find the device and query it directly.

For audio we don't currently have a sensible API for associating
controls with any sort of map of how the device is laid out, userspace
has to play guessing games.

> I think we will need a new ioctl in the media controller API to report
> advanced information about an entity. This could be used to report controls
> implemented by an ALSA element if ALSA doesn't provide a way to do that
> directly. Another use case, which make me think that such an ioctl would be
> needed, is to report UVC extension units type (16-byte GUID) to userspace.

That seems reasonable.

2010-11-25 15:29:42

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

Hi Mark,

On Thursday 25 November 2010 14:41:35 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 10:38:05AM +0100, Clemens Ladisch wrote:
> > In USB and HD audio devices, all links are immutable, and the routing
> > is controlled by 'selector' entities that activate exactly one of their
> > input pads. In userspace, this entity shows up as a mixer control.
> > I guess it would be possible to map the ACTIVE flag onto these controls.
>
> Ditto for ASoC, mostly.
>
> > Alternatively, entities can have 'mute' mixer controls associated with
> > their pads. In this case, multiple unmuted inputs would be mixed
> > together.
> >
> > ALSA has PCM and MIDI devices, and several types of mixer controls.
> > (It also has hardware dependent and timer devices, but I don't think
> >
> > these would need topology information.) So we need at least these:
> > MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
> > MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
> > MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL
> >
> > Furthermore, topology information is also needed for entities not
> > associated with a mixer control, such as microphones, speakers, jacks/
> > connectors, and effect units. These entities are defined in the USB and
> > HD audio specifications, but are not yet handled by ALSA.
>
> All this and more in the embedded case - digital audio link nodes and DSP
> I/O nodes (could possibly do those as digital audio ones) spring to
> mind. Also bear in mind that embedded devices can get *very* large - a
> mobile phone audio system can have of the order of 100 nodes in the
> graph.

It depends on how you define nodes. I can certainly imagine a graph with 100
controls, but maybe several controls can be part of the same node ? On the
video side we've decided to split entities depending on the possible data
paths configurations. As I'm not a fluent ascii-art speaker, please have a
look at pages 4 and 5 of http://www.ideasonboard.org/media/20101103-lpc-
media.pdf

Page 4 shows the internal topology of the OMAP3 ISP. The major blocks in that
diagram are reported as entities. Page 5 shows the internal topology of one of
the blocks, the OMAP3 ISP preview engine. As you can see the pipeline is made
of sub-blocks that implement a single image processing function. As the
pipeline is linear (don't worry about the non-linear part in the beginning,
it's just there to take into account link configurability at the higher level)
we don't export all the sub-blocks as entities, but we expose the controls on
the preview engine entity instead.

> > ALSA devices are not addressed by their device node but with card/device/
> >
> > subdevice numbers; mixer controls have numeric IDs, unique per card:
> > struct {
> >
> > int card;
> > int device;
> > int subdevice;
> >
> > } alsa_device;
> > struct {
> >
> > int card;
> > int numid;
> >
> > } alsa_control;
>
> For the embedded stuff we also have a bunch of stuff in the graph which
> may not be visible to userspace at all at present and would just have a
> string based identifier.

That could be easily added (provided the string is not too long).

--
Regards,

Laurent Pinchart

2010-11-25 15:35:50

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

On Thu, Nov 25, 2010 at 04:29:53PM +0100, Laurent Pinchart wrote:

> It depends on how you define nodes. I can certainly imagine a graph with 100
> controls, but maybe several controls can be part of the same node ? On the
> video side we've decided to split entities depending on the possible data
> paths configurations. As I'm not a fluent ascii-art speaker, please have a
> look at pages 4 and 5 of http://www.ideasonboard.org/media/20101103-lpc-
> media.pdf

Please take a look at:

http://www.wolfsonmicro.com/products/WM8994

for an example of a modern smartphone CODEC. This has in the ballpark
of 75-100 nodes (I've not counted exactly) represented in the routing
graph in the driver today. This does not include any audio routing
available in other components such as the CPU.

2010-11-25 15:40:30

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Mark,

On Thursday 25 November 2010 14:36:50 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 03:28:10AM +0100, Laurent Pinchart wrote:
> > +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.
>
> Is this supposed to reflect the current state (if the link is carrying
> data right now) or if it's possible for the link to carry data?

It's supposed to reflect whether the link can carry data. Think of the active
flag as a valve on a pipe. If the valve is open, the link is active. If the
valve is closed, the link is inactive. This is unrelated to whether water
actually flows through the pipe.

Immutable links have no valve (in theory you could have an inactive immutable
link, but that's not very useful, unless we define immutable as no user-
controllable valve, in which case it might be better to rename it as read-
only, or create separate immutable and read-only flags - just brainstorming
here).

> > +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 revision; /* Entity revision, driver specific */
> > + unsigned long flags; /* Entity flags (MEDIA_ENTITY_FLAG_*) */
> > + u32 group_id; /* Entity group ID */
> > +
> > + u16 num_pads; /* Number of input and output pads */
> > + u16 num_links; /* Number of existing links, both active
> > + * and inactive */
> > + u16 num_backlinks; /* Number of backlinks */
> > + u16 max_links; /* Maximum number of links */
> > +
> > + struct media_pad *pads; /* Pads array (num_pads elements) */
> > + struct media_link *links; /* Links array (max_links elements)*/
>
> Hrm. This is getting kind of large, especially considering the volume
> of data we're holding per node and link already in ASoC. On the other
> hand we may over time be able to refactor some of the existing stuff
> (especially the link management) to use this structure.

That's the idea :-) In the long term (hopefully not that long) I would like to
see both sources of information merged. That's why I'm interested in your
requirements to make sure this will be possible.

--
Regards,

Laurent Pinchart

2010-11-25 15:43:05

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 05/12] media: Reference count and power handling

Hi Mark,

On Thursday 25 November 2010 14:49:08 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 03:28:12AM +0100, Laurent Pinchart wrote:
> > 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.
>
> ASoC has its own power management stuff which doesn't *quite* map onto
> this one as-is. The power determination stuff is essentially identical
> (and this is actually nicer) but we have a separate postprocessing stage
> that actually applies the changes in a sequence which minimises audible
> issues caused by doing things in a bad order (eg, power down a PGA
> before you turn off inputs). This is noddy enough to implement, though
> - we just need a pre and post run notifications to set up and implement
> the changes I think.

That sounds feasible. Sakari, you've implemented power management, what do you
think about this ?

> BTW, I notice you've not CCed the ALSA list, Liam, Takashi or Jaroslav
> on any of this - might be good.

I've just subscrived to the alsa-devel list. I will make sure to CC it, as
well as Liam, Takashi and Jaroslav, on the next version of the patches. Sorry
about the missing CC's in these e-mails.

--
Regards,

Laurent Pinchart

2010-11-25 15:47:33

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 09/12] media: Entity locking and pipeline management

Hi Mark,

On Thursday 25 November 2010 14:53:51 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 03:28:16AM +0100, Laurent Pinchart wrote:
> > Link states must not be modified while streaming is in progress on a
> > graph they belong or connect to. The entity locking API helps drivers
> > enforcing that requirement.
>
> This is not desirable for embedded audio - for example, it is very
> common to wish to switch from headphone to speaker and back mid
> playback, or to move between headset, speakerphone and handset modes on
> a phone. This is normally implemented by reconfiguring the output paths
> at runtime without interrupting applications.

Agreed. The locking should be made optional. I'll work on that.

--
Regards,

Laurent Pinchart

2010-11-25 15:49:55

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart wrote:
> On Thursday 25 November 2010 14:36:50 Mark Brown wrote:
> > On Thu, Nov 25, 2010 at 03:28:10AM +0100, Laurent Pinchart wrote:

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

> > Is this supposed to reflect the current state (if the link is carrying
> > data right now) or if it's possible for the link to carry data?

> It's supposed to reflect whether the link can carry data. Think of the active
> flag as a valve on a pipe. If the valve is open, the link is active. If the
> valve is closed, the link is inactive. This is unrelated to whether water
> actually flows through the pipe.

This seems a confusing name, then - I'd expect an active link to be one
which is actually carrying data rather than one which is available to
carry data. How a more neutrally worded name such as "connected" (which
is what ASoC uses currently)?

This also falls through into the power management stuff, we don't want
to be powering things up unless they're actually doing something right
now.

> Immutable links have no valve (in theory you could have an inactive immutable
> link, but that's not very useful, unless we define immutable as no user-
> controllable valve, in which case it might be better to rename it as read-
> only, or create separate immutable and read-only flags - just brainstorming
> here).

That was what I was expecting immutable to mean - no user control. A
link that's permanantly wired can have the data flow controlled through
its inputs and outputs, even if it is not itself controllable.

2010-11-25 17:20:38

by Andy Walls

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 02/12] media: Media device

The signedness of char is ambiguous for 8 bit data, which is why an API would normally use u8 (or s8, I guess).

Since this is known to be character data, I would think char would be fine. I am assuming C compilers would never assume multibyte "char"s.

Regards,
Andy

Laurent Pinchart <[email protected]> wrote:

>Hi Clemens,
>
>Thanks for the review.
>
>On Thursday 25 November 2010 10:33:02 Clemens Ladisch wrote:
>> Laurent Pinchart wrote:
>> > +struct media_device {
>> > ...
>> > + u8 model[32];
>> > + u8 serial[40];
>> > + u8 bus_info[32];
>>
>> All drivers and userspace applications have to treat this as char[], so
>> why u8[]?
>
>Good question. I've copied the V4L2 practice of using u8 (or __u8) for fixed-
>length strings in structures. I can't think of any reason for that.
>
>I will replace u8 with char unless someone comes up with a good reason to keep
>u8.
>
>--
>Regards,
>
>Laurent Pinchart
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2010-11-25 17:30:29

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 02/12] media: Media device

Hi Andy,

On Thursday 25 November 2010 18:20:31 Andy Walls wrote:
> The signedness of char is ambiguous for 8 bit data, which is why an API
> would normally use u8 (or s8, I guess).
>
> Since this is known to be character data, I would think char would be fine.

I think so too.

> I am assuming C compilers would never assume multibyte "char"s.

Not on sane platforms I guess. We would have problems much worse than MC API
breakage in that case anyway :-)

--
Regards,

Laurent Pinchart

2010-11-25 17:49:14

by Sakari Ailus

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 05/12] media: Reference count and power handling

Laurent Pinchart wrote:
> Hi Mark,

Hi Laurent, others,

> On Thursday 25 November 2010 14:49:08 Mark Brown wrote:
>> On Thu, Nov 25, 2010 at 03:28:12AM +0100, Laurent Pinchart wrote:
>>> 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.
>>
>> ASoC has its own power management stuff which doesn't *quite* map onto
>> this one as-is. The power determination stuff is essentially identical
>> (and this is actually nicer) but we have a separate postprocessing stage
>> that actually applies the changes in a sequence which minimises audible
>> issues caused by doing things in a bad order (eg, power down a PGA
>> before you turn off inputs). This is noddy enough to implement, though
>> - we just need a pre and post run notifications to set up and implement
>> the changes I think.
>
> That sounds feasible. Sakari, you've implemented power management, what do you
> think about this ?

I first have to admit that I don't know ALSA almost at all.

Currently when two media entities are connected they will be powered up
for the duration of the link setup, just in case the drivers would need
to access hardware for some reason. I don't think we have a need for
that at the moment, it's so just because it seemed to be the right thing
to do.

Essentially the idea is that the drivers do not need to be involved with
the power state handling and the device would be powered always when
they are run, but not be powered when it's not necessary.

That feature put aside, then what's left is what Laurent described; the
entities will be powered up based on opening subdev and video nodes
(V4L2 case, for ALSA it'd be different kind of ALSA nodes). But I don't
think there's necessarily even need to remove this feature.

Subdev is a V4L2 specific concept and I don't know if ALSA would benefit
from something similar. If you want to be able to access the individual
enties from user space, then similar arrangement might be useful.

There are use cases to only use subdev nodes on V4L2: camera LED flash
in torch mode, for example. There is no need to power on the rest of the
system.

I don't see a problem in applying restrictions on power on / off
sequence. They would probably be useful also on the V4L2 side in the future.

Could the restrictions be described as dependencies only, i.e. entity 1
power-up requires entity 2 to be powered first, also meaning that entity
2 could be powered down only after entity 1 has been powered down?

This type of restrictions would fit quite well to the current model as
it would essentially mean just media_entity_get() for the dependent
entities, and media_entity_put() when the original entity is powered
down. This would work recursively.

In the case of audio, most if not all devices (right?) would be always
powered up in certain order. At the same time this would be good for the
flash in torch mode case.

Comments, opinions? :-)

Best regards,

--
Sakari Ailus
[email protected]

2010-11-25 18:07:38

by Alan

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 02/12] media: Media device

On Thu, 25 Nov 2010 12:20:31 -0500
Andy Walls <[email protected]> wrote:

> The signedness of char is ambiguous for 8 bit data, which is why an API would normally use u8 (or s8, I guess).
>
> Since this is known to be character data, I would think char would be fine. I am assuming C compilers would never assume multibyte "char"s.

char is 8bit in all modern C. I have used C compilers with configurable 9
or 7 bit char and such a machine is never going to run Linux without
serious insanity. Even grep in 9bit char is not fun...

The advantage of using u8 is that any casting goes the way you expect
whereas when working with UTF-8 things like

int x = name[0];

may not produce the expected result otherwise.

Alan

2010-11-25 21:47:03

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 05/12] media: Reference count and power handling

On Thu, Nov 25, 2010 at 07:49:05PM +0200, Sakari Ailus wrote:

> Currently when two media entities are connected they will be powered up
> for the duration of the link setup, just in case the drivers would need
> to access hardware for some reason. I don't think we have a need for
> that at the moment, it's so just because it seemed to be the right thing
> to do.

This is *really* bad for audio, powering things on and off needlessly
can cause audible effects for users. If the individual drivers need to
do something they can go and implement that but powering things up by
default seems like it'll at best waste power most of the time.

> Essentially the idea is that the drivers do not need to be involved with
> the power state handling and the device would be powered always when
> they are run, but not be powered when it's not necessary.

This is what DAPM does in ASoC at the minute. What it does is that
every time there is a change in the device configuration which might
have affected the power state it walks the graph of power nodes in the
system. If there has been a change in the state the core will generate
a power transition sequence and apply it. The devices have no knowledge
of this (though they can insert manual sequences for nodes if they need
to), for the most part they just describe the graph and the register
bits that control the power for the nodes and the routing.

> Subdev is a V4L2 specific concept and I don't know if ALSA would benefit
> from something similar. If you want to be able to access the individual
> enties from user space, then similar arrangement might be useful.

ALSA already has this feature (at least in the embedded case, HDA has
some of these features too).

> I don't see a problem in applying restrictions on power on / off
> sequence. They would probably be useful also on the V4L2 side in the future.

> Could the restrictions be described as dependencies only, i.e. entity 1
> power-up requires entity 2 to be powered first, also meaning that entity
> 2 could be powered down only after entity 1 has been powered down?

No. Audio power sequencing tends to work better if sorted by the type
of object rather than the route (though using the route as a lower order
sort key can be useful). It's also useful to coalesce the I/O on the
hardware for performance (both speed and user experience).

The way we're currently doing it the sequencing is actually separated
from the graph walk - the result of the graph walk is a set of things
that need doing which is then implemented in a separate step. I think
this would be the easiest way to integrate with what you're doing at the
minute, keep the same split and then ASoC can do the postprocessing of
the results of the graph walk in the same way as it does currently.

2010-11-26 09:08:58

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] [alsa-devel] media: Entities, pads and links

Laurent Pinchart wrote:
> On Thursday 25 November 2010 10:38:05 Clemens Ladisch wrote:
> > MEDIA_ENTITY_TYPE_NODE_ALSA_PCM
> > MEDIA_ENTITY_TYPE_NODE_ALSA_MIDI
> > MEDIA_ENTITY_TYPE_SUBDEV_ALSA_CONTROL
>
> I agree about PCM and MIDI, but I'm not sure about controls. If controls are
> part of an entity, the entity will be reported through the media controller
> API. If information about that entity can be queried through existing APIs
> (ALSA, V4L, ...) those APIs should be used.

At the moment, ALSA has no API for topology information.

> I can certainly imagine a graph with 100 controls, but maybe several
> controls can be part of the same node ?

There is indeed no strict 1:1 relation; e.g., volume and mute are often
part of the same node. So it looks we need some kind of separate ALSA
node, which can be associated with mixer controls and/or other
information.

ALSA already has is a method to query arbitrary (TLV) metadata for mixer
controls; the entity/control relationship can be stored in the control.

> I think we will need a new ioctl in the media controller API to report
> advanced information about an entity. This could be used to report controls
> implemented by an ALSA element if ALSA doesn't provide a way to do that
> directly.

This advanced information would always be specific to the entity type,
so maybe this should be part of that subsystem's API. Otherwise, the
media_entity would need a callback, or store a pointer to some memory
block (which assumes that the information is always constant).

> > Furthermore, topology information is also needed for entities not
> > associated with a mixer control, such as microphones, speakers, jacks/
> > connectors, and effect units. These entities are defined in the USB and
> > HD audio specifications, but are not yet handled by ALSA.
>
> Agreed, we will need to add new entity types and subtypes for those. The
> reason they're not part of this submission is that I wanted real use cases
> instead of coming up with a made-up list of entities I think would be useful.

The reason that I'm always mentioning the USB and HD audio specs is that
those already define entities that should cover practically all of the
audio needs. (And, of course, we want to be able to report those
entities without having to do too many conversions.)

I'll see if I can draw up the ALSA-specific media stuff over the weekend.

Regards,
Clemens

2010-11-26 14:07:37

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 00/12] Media controller (core and V4L2)

Hi Mark,

On Thursday 25 November 2010 15:28:04 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 03:28:07AM +0100, Laurent Pinchart wrote:
> > I want to emphasize that the media controller API does *not* replace the
> > V4L, DVB or ALSA APIs. It complements them.
>
> Overall this looks relatively good and should be mappable onto the
> embedded audio stack.

Thanks a lot for the review.

> I'd need to sit down and actually do that to go into too much specific
> detail but the only real issue I see is working out how the control aspects
> of this interact with the control in ALSA. The simplest thing would be to
> have the userspace API be read only for ALSA devices, I guess.

That sounds good, especially for the first step.

--
Regards,

Laurent Pinchart

2010-11-26 14:13:23

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Mark,

On Thursday 25 November 2010 16:49:52 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart wrote:
> > On Thursday 25 November 2010 14:36:50 Mark Brown wrote:
> > > On Thu, Nov 25, 2010 at 03:28:10AM +0100, Laurent Pinchart wrote:
> > > > + 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.
> > >
> > > Is this supposed to reflect the current state (if the link is carrying
> > > data right now) or if it's possible for the link to carry data?
> >
> > It's supposed to reflect whether the link can carry data. Think of the
> > active flag as a valve on a pipe. If the valve is open, the link is
> > active. If the valve is closed, the link is inactive. This is unrelated
> > to whether water actually flows through the pipe.
>
> This seems a confusing name, then - I'd expect an active link to be one
> which is actually carrying data rather than one which is available to
> carry data. How a more neutrally worded name such as "connected" (which
> is what ASoC uses currently)?

In our current vocabulary "connected" refers to entities between which a link
exist, regardless of the link state ("valve opened" or "valve closed"). I'm
not totally happy with "active" either, but if we replace it with "connected"
we need another word to replace current uses of "connected".

> This also falls through into the power management stuff, we don't want
> to be powering things up unless they're actually doing something right
> now.
>
> > Immutable links have no valve (in theory you could have an inactive
> > immutable link, but that's not very useful, unless we define immutable
> > as no user- controllable valve, in which case it might be better to
> > rename it as read- only, or create separate immutable and read-only
> > flags - just brainstorming here).
>
> That was what I was expecting immutable to mean - no user control. A
> link that's permanantly wired can have the data flow controlled through
> its inputs and outputs, even if it is not itself controllable.

--
Regards,

Laurent Pinchart

2010-11-26 14:14:45

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Fri, Nov 26, 2010 at 03:13:36PM +0100, Laurent Pinchart wrote:
> On Thursday 25 November 2010 16:49:52 Mark Brown wrote:
> > On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart wrote:

> > > It's supposed to reflect whether the link can carry data. Think of the
> > > active flag as a valve on a pipe. If the valve is open, the link is
> > > active. If the valve is closed, the link is inactive. This is unrelated
> > > to whether water actually flows through the pipe.

> > This seems a confusing name, then - I'd expect an active link to be one
> > which is actually carrying data rather than one which is available to
> > carry data. How a more neutrally worded name such as "connected" (which
> > is what ASoC uses currently)?

> In our current vocabulary "connected" refers to entities between which a link
> exist, regardless of the link state ("valve opened" or "valve closed"). I'm
> not totally happy with "active" either, but if we replace it with "connected"
> we need another word to replace current uses of "connected".

Linked?

2010-11-28 15:48:46

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 05/12] media: Reference count and power handling

Hi Mark,

On Thursday 25 November 2010 22:47:09 Mark Brown wrote:
> On Thu, Nov 25, 2010 at 07:49:05PM +0200, Sakari Ailus wrote:
> > Currently when two media entities are connected they will be powered up
> > for the duration of the link setup, just in case the drivers would need
> > to access hardware for some reason. I don't think we have a need for
> > that at the moment, it's so just because it seemed to be the right thing
> > to do.
>
> This is *really* bad for audio, powering things on and off needlessly
> can cause audible effects for users. If the individual drivers need to
> do something they can go and implement that but powering things up by
> default seems like it'll at best waste power most of the time.

The OMAP3 ISP driver required entities to be powered on when link attributes
were modified, but that's not true anymore. We could remove this feature, or
make it optional.

> > Essentially the idea is that the drivers do not need to be involved with
> > the power state handling and the device would be powered always when
> > they are run, but not be powered when it's not necessary.
>
> This is what DAPM does in ASoC at the minute. What it does is that
> every time there is a change in the device configuration which might
> have affected the power state it walks the graph of power nodes in the
> system. If there has been a change in the state the core will generate
> a power transition sequence and apply it. The devices have no knowledge
> of this (though they can insert manual sequences for nodes if they need
> to), for the most part they just describe the graph and the register
> bits that control the power for the nodes and the routing.
>
> > Subdev is a V4L2 specific concept and I don't know if ALSA would benefit
> > from something similar. If you want to be able to access the individual
> > enties from user space, then similar arrangement might be useful.
>
> ALSA already has this feature (at least in the embedded case, HDA has
> some of these features too).
>
> > I don't see a problem in applying restrictions on power on / off
> > sequence. They would probably be useful also on the V4L2 side in the
> > future.
> >
> > Could the restrictions be described as dependencies only, i.e. entity 1
> > power-up requires entity 2 to be powered first, also meaning that entity
> > 2 could be powered down only after entity 1 has been powered down?
>
> No. Audio power sequencing tends to work better if sorted by the type
> of object rather than the route (though using the route as a lower order
> sort key can be useful). It's also useful to coalesce the I/O on the
> hardware for performance (both speed and user experience).
>
> The way we're currently doing it the sequencing is actually separated
> from the graph walk - the result of the graph walk is a set of things
> that need doing which is then implemented in a separate step. I think
> this would be the easiest way to integrate with what you're doing at the
> minute, keep the same split and then ASoC can do the postprocessing of
> the results of the graph walk in the same way as it does currently.

Throwing an idea here, what about making power management modular ? The MC
core would then call back to drivers to handle use count changes. The
media_entity_use_change() function would be exported (and renamed), so that
drivers could use it if they want graph-based power management. ALSA would
have its own use count handler.

--
Regards,

Laurent Pinchart

2010-11-28 15:48:57

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Mark,

On Friday 26 November 2010 15:14:42 Mark Brown wrote:
> On Fri, Nov 26, 2010 at 03:13:36PM +0100, Laurent Pinchart wrote:
> > On Thursday 25 November 2010 16:49:52 Mark Brown wrote:
> > > On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart wrote:
> > > > It's supposed to reflect whether the link can carry data. Think of
> > > > the active flag as a valve on a pipe. If the valve is open, the link
> > > > is active. If the valve is closed, the link is inactive. This is
> > > > unrelated to whether water actually flows through the pipe.
> > >
> > > This seems a confusing name, then - I'd expect an active link to be one
> > > which is actually carrying data rather than one which is available to
> > > carry data. How a more neutrally worded name such as "connected"
> > > (which is what ASoC uses currently)?
> >
> > In our current vocabulary "connected" refers to entities between which a
> > link exist, regardless of the link state ("valve opened" or "valve
> > closed"). I'm not totally happy with "active" either, but if we replace
> > it with "connected" we need another word to replace current uses of
> > "connected".
>
> Linked?

That's a good option. Hans, do you want to comment on this ?

--
Regards,

Laurent Pinchart

2010-11-28 15:57:31

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Sunday, November 28, 2010 13:34:45 Laurent Pinchart wrote:
> Hi Mark,
>
> On Friday 26 November 2010 15:14:42 Mark Brown wrote:
> > On Fri, Nov 26, 2010 at 03:13:36PM +0100, Laurent Pinchart wrote:
> > > On Thursday 25 November 2010 16:49:52 Mark Brown wrote:
> > > > On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart wrote:
> > > > > It's supposed to reflect whether the link can carry data. Think of
> > > > > the active flag as a valve on a pipe. If the valve is open, the link
> > > > > is active. If the valve is closed, the link is inactive. This is
> > > > > unrelated to whether water actually flows through the pipe.
> > > >
> > > > This seems a confusing name, then - I'd expect an active link to be one
> > > > which is actually carrying data rather than one which is available to
> > > > carry data. How a more neutrally worded name such as "connected"
> > > > (which is what ASoC uses currently)?
> > >
> > > In our current vocabulary "connected" refers to entities between which a
> > > link exist, regardless of the link state ("valve opened" or "valve
> > > closed"). I'm not totally happy with "active" either, but if we replace
> > > it with "connected" we need another word to replace current uses of
> > > "connected".
> >
> > Linked?
>
> That's a good option. Hans, do you want to comment on this ?
>
>

Fine by me! It's better than 'active'.

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by Cisco

2010-11-28 18:25:47

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 05/12] media: Reference count and power handling

On 28 Nov 2010, at 12:33, Laurent Pinchart wrote:

> Throwing an idea here, what about making power management modular ? The MC
> core would then call back to drivers to handle use count changes. The
> media_entity_use_change() function would be exported (and renamed), so that
> drivers could use it if they want graph-based power management. ALSA would
> have its own use count handler.

We need more than just the count handler - we need a complete list of everything that has been changed in the whole system. The graph walk itself is fine but we need to be able to do postprocessing of the delta that is generated. Like I said pre- and post- callbacks may well cover it.

Note that this is specifically for ASoC, not ALSA in general.-

2010-12-03 13:50:35

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Hans,

Adding by the original CC list which was dropped by mistake.

On Friday 03 December 2010 13:06:18 Hans Verkuil wrote:
> On Friday, December 03, 2010 11:19:36 Laurent Pinchart wrote:
> > On Sunday 28 November 2010 16:57:00 you wrote:
> > > On Sunday, November 28, 2010 13:34:45 Laurent Pinchart wrote:
> > > > On Friday 26 November 2010 15:14:42 Mark Brown wrote:
> > > > > On Fri, Nov 26, 2010 at 03:13:36PM +0100, Laurent Pinchart wrote:
> > > > > > On Thursday 25 November 2010 16:49:52 Mark Brown wrote:
> > > > > > > On Thu, Nov 25, 2010 at 04:40:41PM +0100, Laurent Pinchart
wrote:
> > > > > > > > It's supposed to reflect whether the link can carry data.
> > > > > > > > Think of the active flag as a valve on a pipe. If the valve
> > > > > > > > is open, the link is active. If the valve is closed, the
> > > > > > > > link is inactive. This is unrelated to whether water
> > > > > > > > actually flows through the pipe.
> > > > > > >
> > > > > > > This seems a confusing name, then - I'd expect an active link
> > > > > > > to be one which is actually carrying data rather than one
> > > > > > > which is available to carry data. How a more neutrally worded
> > > > > > > name such as "connected" (which is what ASoC uses currently)?
> > > > > >
> > > > > > In our current vocabulary "connected" refers to entities between
> > > > > > which a link exist, regardless of the link state ("valve opened"
> > > > > > or "valve closed"). I'm not totally happy with "active" either,
> > > > > > but if we replace it with "connected" we need another word to
> > > > > > replace current uses of "connected".
> > > > >
> > > > > Linked?
> > > >
> > > > That's a good option. Hans, do you want to comment on this ?
> > >
> > > Fine by me! It's better than 'active'.
> >
> > Just to confirm thinks, Mark's proposal is to replace 'connected' by
> > 'linked' and 'active' by 'connected'. Are we on the same page here ?
>
> Yes, but when I read it back it does not make me happy. 'Connected' and
> 'linked' basically have the same meaning in English.

I unfortunately agree that it's a bit confusing :-(

> I really like your analogy with valves, so perhaps we should use either
> 'linked' or 'connected' to describe that two entities are, well,
> linked/connected, and use the 'open' and 'closed' terminology to describe
> whether a link/connection is open (data can flow) or closed (no data can
> flow).

I don't really like the open/closed terminology to describe links.

I can think of two analogies: pipes with valves that can be opened/closed, or
cables that can be connected/disconnected.

In the first case, connected/linked can be used to specify the pipes that
exist in the system, but I'm not happy with open/closed.

In the second case, connected/linked can be used to specify whether a cable is
connected, but in that case we will need another word to describe whether two
pads are connectable or not.

> I have a slight preference for 'link' over 'connection', but that's mostly
> because it is a shorter word :-)

Link refers to a pipe/possible cable connection. It's an object on both the
kernel side and the userspace side. Using the above analogies, tt makes sense
to use the word 'linked' to refer to two pads that are connected by a pipe, or
between which a cable can be connected.

Now we need a word to descripe whether the valve is opened or closed, or
whether the cable is connected or not. I don't really like 'open'/'closed' for
the first analogy. 'connected' would make sense for the second analogy, but it
can indeed be a bit confusing.

Thoughts ?

--
Regards,

Laurent Pinchart

2010-12-03 14:54:12

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Fri, Dec 03, 2010 at 02:50:58PM +0100, Laurent Pinchart wrote:
> On Friday 03 December 2010 13:06:18 Hans Verkuil wrote:

> > > Just to confirm thinks, Mark's proposal is to replace 'connected' by
> > > 'linked' and 'active' by 'connected'. Are we on the same page here ?

> > Yes, but when I read it back it does not make me happy. 'Connected' and
> > 'linked' basically have the same meaning in English.

> I unfortunately agree that it's a bit confusing :-(

It feels like the problem here is that for whatever reason (I'm not sure
what?) you're trying to come up with verbs for links that are currently
disconnected - in ASoC we just say we've got paths that exist and then
we talk about the paths that are connected. Verbing everything makes it
all sound active which is confusing when you're talking about links that
are idle.

2010-12-07 17:13:59

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Friday, December 03, 2010 15:54:08 Mark Brown wrote:
> On Fri, Dec 03, 2010 at 02:50:58PM +0100, Laurent Pinchart wrote:
> > On Friday 03 December 2010 13:06:18 Hans Verkuil wrote:
>
> > > > Just to confirm thinks, Mark's proposal is to replace 'connected' by
> > > > 'linked' and 'active' by 'connected'. Are we on the same page here ?
>
> > > Yes, but when I read it back it does not make me happy. 'Connected' and
> > > 'linked' basically have the same meaning in English.
>
> > I unfortunately agree that it's a bit confusing :-(
>
> It feels like the problem here is that for whatever reason (I'm not sure
> what?) you're trying to come up with verbs for links that are currently
> disconnected - in ASoC we just say we've got paths that exist and then
> we talk about the paths that are connected. Verbing everything makes it
> all sound active which is confusing when you're talking about links that
> are idle.

OK, let's try this again.

The media controller has entities, entities have pads, and between pads there
are links.

Links can be active (data can flow) or inactive (no data can flow).

Active links can be idle (no data is flowing) or streaming (data is flowing
over the link).

Personally I think this is perfectly clear. The original confusion came from
the word 'active', which I understand means 'streaming' in alsa. By adding
a 'streaming' flag in addition to the active flag I think it will be clear
that 'active' and 'streaming' are two different things.

Regarding 'active': an alternative could be 'connected'. I think it is not
quite as good as 'active' since basically all links are always connected in
the usual sense of the word. It is just that a mux decides which one is
actually working. However, I won't object to using 'connected' instead of
'active' if others prefer that.

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by Cisco

2010-12-07 17:55:09

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Tue, Dec 07, 2010 at 06:13:35PM +0100, Hans Verkuil wrote:

> Personally I think this is perfectly clear. The original confusion came from
> the word 'active', which I understand means 'streaming' in alsa. By adding
> a 'streaming' flag in addition to the active flag I think it will be clear
> that 'active' and 'streaming' are two different things.

It's not that, it's the fact that active sounds to me like the link
ought to be powered up which isn't the case - it's too verby. Like I
say, I think I'd prefer a more passive name like "connected".

I guess if nobody else finds it confusing I can live with it.

2010-12-07 18:11:50

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Tuesday, December 07, 2010 18:55:05 Mark Brown wrote:
> On Tue, Dec 07, 2010 at 06:13:35PM +0100, Hans Verkuil wrote:
>
> > Personally I think this is perfectly clear. The original confusion came from
> > the word 'active', which I understand means 'streaming' in alsa. By adding
> > a 'streaming' flag in addition to the active flag I think it will be clear
> > that 'active' and 'streaming' are two different things.
>
> It's not that, it's the fact that active sounds to me like the link
> ought to be powered up which isn't the case - it's too verby. Like I
> say, I think I'd prefer a more passive name like "connected".

Ah, now I understand what you mean. Would 'activated' be better than 'active'?

Or perhaps just say: the link 'is on' or the link 'is switched on'?

So: ...LINK_SWITCHED_ON (sorry, forgot what the prefix is).

Actually, I think 'switched on' is a pretty good description of what is going on
in the hardware.

Regards,

Hans

> I guess if nobody else finds it confusing I can live with it.
>

--
Hans Verkuil - video4linux developer - sponsored by Cisco

2010-12-07 19:03:32

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

On Tue, Dec 07, 2010 at 07:11:39PM +0100, Hans Verkuil wrote:

> Ah, now I understand what you mean. Would 'activated' be better than 'active'?

Better, yes, though it still sounds a bit like something should be
actively (IYSWIM) happening. In the absence of better ideas I could go
with this.

> Or perhaps just say: the link 'is on' or the link 'is switched on'?

> So: ...LINK_SWITCHED_ON (sorry, forgot what the prefix is).

> Actually, I think 'switched on' is a pretty good description of what is going on
> in the hardware.

I prefer activated, this makes me think of power. Bear in mind that for
most audio the power is a big portion of the control - either the audio
is analogue or it looks like it.

2010-12-09 12:52:38

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi,

On Tuesday 07 December 2010 20:03:28 Mark Brown wrote:
> On Tue, Dec 07, 2010 at 07:11:39PM +0100, Hans Verkuil wrote:
> > Ah, now I understand what you mean. Would 'activated' be better than
> > 'active'?
>
> Better, yes, though it still sounds a bit like something should be
> actively (IYSWIM) happening. In the absence of better ideas I could go
> with this.
>
> > Or perhaps just say: the link 'is on' or the link 'is switched on'?
> >
> > So: ...LINK_SWITCHED_ON (sorry, forgot what the prefix is).
> >
> > Actually, I think 'switched on' is a pretty good description of what is
> > going on in the hardware.
>
> I prefer activated, this makes me think of power. Bear in mind that for
> most audio the power is a big portion of the control - either the audio
> is analogue or it looks like it.

Do I understand it correctly that the outcome of this discussion is that I
just need to rename active to activated when talking about links ?

--
Regards,

Laurent Pinchart

2010-12-10 16:35:12

by Sakari Ailus

[permalink] [raw]

Subject: Re: [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Mark and others!

Mark Brown wrote:
> On Tue, Dec 07, 2010 at 07:11:39PM +0100, Hans Verkuil wrote:
>
>> Ah, now I understand what you mean. Would 'activated' be better than 'active'?
>
> Better, yes, though it still sounds a bit like something should be
> actively (IYSWIM) happening. In the absence of better ideas I could go
> with this.

Activated, to me, sounds like that it has happened as a consequence of
something, and primarily not as a result of a user request.

This flag is also set using MEDIA_IOC_SETUP_LINK ioctl and setting the
flags field would be something like this:

media_link_desc.flags |= MEDIA_LINK_ACTIVATED;

I'd prefer ACTIVE over ACTIVATED, since this may also be set (and is
mostly set) by the user.

But reading this discussion, ACTIVE has not received unanimous approval
either... :\

>> Or perhaps just say: the link 'is on' or the link 'is switched on'?
>
>> So: ...LINK_SWITCHED_ON (sorry, forgot what the prefix is).
>
>> Actually, I think 'switched on' is a pretty good description of what is going on
>> in the hardware.
>
> I prefer activated, this makes me think of power. Bear in mind that for
> most audio the power is a big portion of the control - either the audio
> is analogue or it looks like it.

What would you think about "ENABLED"? It's simple, quite generic and
thus doesn't explicitly suggest what is the exact effect it has on the
level of the underlying device.

I don't completely like it myself since it would be best to have an
adjective (like "active") but there's none for the word enable, so it
bears the same issues than "activated".

Cheers,

--
Sakari Ailus
[email protected]

2010-12-13 16:08:09

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

I wrote:
> I'll see if I can draw up the ALSA-specific media stuff over the weekend.

Sorry, wrong weekend.

Anyway, below are some remarks and a patch.

* Entity types

TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
in a graph, which does not distinguish it from other entity types
because all entities are part of the topology graph. I chose "device"
as this type describes entities that are visible as some device node to
other software.

TYPE_EXT describes entities that represent some interface to the
external world, TYPE_INT those that are internal to the entire device.
(I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
to be an even more meaningless name.)

ALSA mixer controls are not directly represented; a better fit for the
architecture of actual devices is that one or more mixer controls can be
associated with an entity. (This can be done with a field of the mixer
control.)

* Entity properties

There needs to be a mechanism to associate meta-information (properties)
with entities. This information should be optional and extensible, but,
when being handled inside the kernel, doesn't need to be more than
a read-only blob. I think that something like ALSA's TLV format (used
for mixer controls) can be used here. (I'm not mentioning the X-word
here, except to note that the "M" stands for "markup".)

* Entity subtypes

EXT_JACK_ANALOG represents any analog audio and/or video connector.
Properties for audio jacks would be jack type (TRS/RCA), color code,
line level, position, etc.

EXT_JACK_DIGITAL represents a digital connector like S/PDIF (coax/
TOSLINK), ADAT, TDIF, or MADI.

EXT_JACK_BUS represents a bus like FireWire and comes from the USB audio
spec. (I doubt that any devices with this entitiy will ever exist.)

EXT_INSTRUMENT represents something like an e-guitar, keyboard, or MIDI
controller. (Instrument entities are typically audio sources and MIDI
sources and sinks, but can also be audio sinks.)

EXT_SPEAKER also includes headphones; there might be made a case for
having those as a separate subtype.

EXT_PLAYER represents a device like a CD/DVD/tape player. Recorders can
also write to that device, so "player" might not be an ideal name.

EXT_BROADCAST represents devices like TV tuners, satellite receivers,
cable tuners, or radios.

INT_SYNTHESIZER converts MIDI to audio.

INT_NOISE_SOURCE comes from the USB audio spec; this is not an attempt
to describe the characteristics of consumer-grade devices :-) , but
represents an internal noise source for level calibration or measurements.

INT_CONTROLS may have multiple independent controls (this is USB's
Feature Unit); INT_EFFECT may have multiple controls that affect one
single algorithm.

INT_CHANNEL_SPLIT/MERGE are needed for HDAudio devices, whose topology
information has only stereo links.

* Entity specifications

While TYPE_DEVICE entities can be identified by their device node, other
entities typcially have just a numeric ID. For that, it would be useful
to make do without separate identification and let the driver choose the
entity ID.

Signed-off-by: Clemens Ladisch <[email protected]>

--- linux/include/linux/media.h
+++ linux/include/linux/media.h
@@ -46,16 +46,36 @@ struct media_device_info {
#define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
#define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff

-#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
-#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
-#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
-#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
-#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
+#define MEDIA_ENTITY_TYPE_DEVICE (1 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_DEVICE_V4L (MEDIA_ENTITY_TYPE_DEVICE + 1)
+#define MEDIA_ENTITY_TYPE_DEVICE_FB (MEDIA_ENTITY_TYPE_DEVICE + 2)
+#define MEDIA_ENTITY_TYPE_DEVICE_DVB (MEDIA_ENTITY_TYPE_DEVICE + 3)
+#define MEDIA_ENTITY_TYPE_DEVICE_ALSA_PCM (MEDIA_ENTITY_TYPE_DEVICE + 4)
+#define MEDIA_ENTITY_TYPE_DEVICE_ALSA_MIDI (MEDIA_ENTITY_TYPE_DEVICE + 5)

-#define MEDIA_ENTITY_TYPE_SUBDEV (2 << MEDIA_ENTITY_TYPE_SHIFT)
-#define MEDIA_ENTITY_TYPE_SUBDEV_SENSOR (MEDIA_ENTITY_TYPE_SUBDEV + 1)
-#define MEDIA_ENTITY_TYPE_SUBDEV_FLASH (MEDIA_ENTITY_TYPE_SUBDEV + 2)
-#define MEDIA_ENTITY_TYPE_SUBDEV_LENS (MEDIA_ENTITY_TYPE_SUBDEV + 3)
+#define MEDIA_ENTITY_TYPE_EXT (2 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_EXT_SENSOR (MEDIA_ENTITY_TYPE_EXT + 1)
+#define MEDIA_ENTITY_TYPE_EXT_FLASH (MEDIA_ENTITY_TYPE_EXT + 2)
+#define MEDIA_ENTITY_TYPE_EXT_LENS (MEDIA_ENTITY_TYPE_EXT + 3)
+#define MEDIA_ENTITY_TYPE_EXT_JACK_MIDI (MEDIA_ENTITY_TYPE_EXT + 4)
+#define MEDIA_ENTITY_TYPE_EXT_JACK_ANALOG (MEDIA_ENTITY_TYPE_EXT + 5)
+#define MEDIA_ENTITY_TYPE_EXT_JACK_DIGITAL (MEDIA_ENTITY_TYPE_EXT + 6)
+#define MEDIA_ENTITY_TYPE_EXT_JACK_BUS (MEDIA_ENTITY_TYPE_EXT + 7)
+#define MEDIA_ENTITY_TYPE_EXT_INSTRUMENT (MEDIA_ENTITY_TYPE_EXT + 8)
+#define MEDIA_ENTITY_TYPE_EXT_SPEAKER (MEDIA_ENTITY_TYPE_EXT + 9)
+#define MEDIA_ENTITY_TYPE_EXT_MICROPHONE (MEDIA_ENTITY_TYPE_EXT + 10)
+#define MEDIA_ENTITY_TYPE_EXT_PLAYER (MEDIA_ENTITY_TYPE_EXT + 11)
+#define MEDIA_ENTITY_TYPE_EXT_BROADCAST (MEDIA_ENTITY_TYPE_EXT + 12)
+
+#define MEDIA_ENTITY_TYPE_INT (3 << MEDIA_ENTITY_TYPE_SHIFT)
+#define MEDIA_ENTITY_TYPE_INT_SYNTHESIZER (MEDIA_ENTITY_TYPE_INT + 1)
+#define MEDIA_ENTITY_TYPE_INT_NOISE_SOURCE (MEDIA_ENTITY_TYPE_INT + 2)
+#define MEDIA_ENTITY_TYPE_INT_MIXER (MEDIA_ENTITY_TYPE_INT + 3)
+#define MEDIA_ENTITY_TYPE_INT_SELECTOR (MEDIA_ENTITY_TYPE_INT + 4)
+#define MEDIA_ENTITY_TYPE_INT_CONTROLS (MEDIA_ENTITY_TYPE_INT + 5)
+#define MEDIA_ENTITY_TYPE_INT_EFFECT (MEDIA_ENTITY_TYPE_INT + 6)
+#define MEDIA_ENTITY_TYPE_INT_CHANNEL_SPLIT (MEDIA_ENTITY_TYPE_INT + 7)
+#define MEDIA_ENTITY_TYPE_INT_CHANNEL_MERGE (MEDIA_ENTITY_TYPE_INT + 8)

#define MEDIA_ENTITY_FLAG_DEFAULT (1 << 0)

@@ -72,7 +92,7 @@ struct media_entity_desc {
__u32 reserved[4];

union {
- /* Node specifications */
+ /* Device specifications */
struct {
__u32 major;
__u32 minor;
@@ -81,11 +101,15 @@ struct media_entity_desc {
__u32 major;
__u32 minor;
} fb;
- int alsa;
+ struct {
+ __u32 card;
+ __u32 device;
+ __s32 subdevice;
+ } alsa;
int dvb;

/* Sub-device specifications */
/* Nothing needed yet */
__u8 raw[184];
};
};

2010-12-14 12:00:06

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Clemens,

On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> I wrote:
> > I'll see if I can draw up the ALSA-specific media stuff over the weekend.
>
> Sorry, wrong weekend.
>
> Anyway, below are some remarks and a patch.

Thank you. Please see my comments inline.

> * Entity types
>
> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
> in a graph, which does not distinguish it from other entity types
> because all entities are part of the topology graph. I chose "device"
> as this type describes entities that are visible as some device node to
> other software.

What this type describes is a device node. Both NODE and DEVICE can be
confusing in my opinion, but DEVICE_NODE is a bit long.

> TYPE_EXT describes entities that represent some interface to the
> external world, TYPE_INT those that are internal to the entire device.
> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> to be an even more meaningless name.)

SUBDEV comes from the V4L2 world, and I agree that it might not be a very good
name.

I'm not sure I would split entities in internal/external categories. I would
create a category for connectors though.

> ALSA mixer controls are not directly represented; a better fit for the
> architecture of actual devices is that one or more mixer controls can be
> associated with an entity. (This can be done with a field of the mixer
> control.)

Agreed.

> * Entity properties
>
> There needs to be a mechanism to associate meta-information (properties)
> with entities. This information should be optional and extensible, but,
> when being handled inside the kernel, doesn't need to be more than
> a read-only blob. I think that something like ALSA's TLV format (used
> for mixer controls) can be used here. (I'm not mentioning the X-word
> here, except to note that the "M" stands for "markup".)

I've been thinking of adding a new ioctl for that. It's something we need to
draft. The UVC driver will need it, and I'm pretty sure other V4L2 drivers
would find it useful as well.

> * Entity subtypes
>
> EXT_JACK_ANALOG represents any analog audio and/or video connector.
> Properties for audio jacks would be jack type (TRS/RCA), color code,
> line level, position, etc.
>
> EXT_JACK_DIGITAL represents a digital connector like S/PDIF (coax/
> TOSLINK), ADAT, TDIF, or MADI.
>
> EXT_JACK_BUS represents a bus like FireWire and comes from the USB audio
> spec. (I doubt that any devices with this entitiy will ever exist.)
>
> EXT_INSTRUMENT represents something like an e-guitar, keyboard, or MIDI
> controller. (Instrument entities are typically audio sources and MIDI
> sources and sinks, but can also be audio sinks.)
>
> EXT_SPEAKER also includes headphones; there might be made a case for
> having those as a separate subtype.

Shouldn't headphones be represented by an EXT_JACK_ANALOG ?

> EXT_PLAYER represents a device like a CD/DVD/tape player. Recorders can
> also write to that device, so "player" might not be an ideal name.
>
> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
> cable tuners, or radios.

There's clearly an overlap with V4L here. Hopefully someone from the linux-
media list can comment on this.

> INT_SYNTHESIZER converts MIDI to audio.
>
> INT_NOISE_SOURCE comes from the USB audio spec; this is not an attempt
> to describe the characteristics of consumer-grade devices :-) , but
> represents an internal noise source for level calibration or measurements.
>
> INT_CONTROLS may have multiple independent controls (this is USB's
> Feature Unit); INT_EFFECT may have multiple controls that affect one
> single algorithm.

I'd describe this as a feature unit/processing unit then.

> INT_CHANNEL_SPLIT/MERGE are needed for HDAudio devices, whose topology
> information has only stereo links.

Some of those INT entities could also be implemented in dedicated chips, so I
really think the EXT/INT split doesn't make too much sense. Should we have an
AUDIO category ?

> * Entity specifications
>
> While TYPE_DEVICE entities can be identified by their device node, other
> entities typcially have just a numeric ID.

In V4L2 sub-devices have (or rather will have once the media controller
patches will be integrated) device nodes as well, so exposing that information
is required.

> For that, it would be useful to make do without separate identification and
> let the driver choose the entity ID.

How would drivers do that ? What if you have two instances of the same chip (a
video sensor, audio mixer, ...) on the same board ?

> Signed-off-by: Clemens Ladisch <[email protected]>
>
> --- linux/include/linux/media.h
> +++ linux/include/linux/media.h
> @@ -46,16 +46,36 @@ struct media_device_info {
> #define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
> #define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
>
> -#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
> -#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
> -#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
> -#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
> -#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
> +#define MEDIA_ENTITY_TYPE_DEVICE (1 << MEDIA_ENTITY_TYPE_SHIFT)
> +#define MEDIA_ENTITY_TYPE_DEVICE_V4L (MEDIA_ENTITY_TYPE_DEVICE + 1)
> +#define MEDIA_ENTITY_TYPE_DEVICE_FB (MEDIA_ENTITY_TYPE_DEVICE + 2)
> +#define MEDIA_ENTITY_TYPE_DEVICE_DVB (MEDIA_ENTITY_TYPE_DEVICE + 3)
> +#define MEDIA_ENTITY_TYPE_DEVICE_ALSA_PCM (MEDIA_ENTITY_TYPE_DEVICE + 4)
> +#define MEDIA_ENTITY_TYPE_DEVICE_ALSA_MIDI (MEDIA_ENTITY_TYPE_DEVICE + 5)
>
> -#define MEDIA_ENTITY_TYPE_SUBDEV (2 << MEDIA_ENTITY_TYPE_SHIFT)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_SENSOR (MEDIA_ENTITY_TYPE_SUBDEV + 1)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_FLASH (MEDIA_ENTITY_TYPE_SUBDEV + 2)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_LENS (MEDIA_ENTITY_TYPE_SUBDEV + 3)
> +#define MEDIA_ENTITY_TYPE_EXT (2 << MEDIA_ENTITY_TYPE_SHIFT)
> +#define MEDIA_ENTITY_TYPE_EXT_SENSOR (MEDIA_ENTITY_TYPE_EXT + 1)
> +#define MEDIA_ENTITY_TYPE_EXT_FLASH (MEDIA_ENTITY_TYPE_EXT + 2)
> +#define MEDIA_ENTITY_TYPE_EXT_LENS (MEDIA_ENTITY_TYPE_EXT + 3)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_MIDI (MEDIA_ENTITY_TYPE_EXT + 4)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_ANALOG (MEDIA_ENTITY_TYPE_EXT + 5)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_DIGITAL (MEDIA_ENTITY_TYPE_EXT + 6)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_BUS (MEDIA_ENTITY_TYPE_EXT + 7)
> +#define MEDIA_ENTITY_TYPE_EXT_INSTRUMENT (MEDIA_ENTITY_TYPE_EXT + 8)
> +#define MEDIA_ENTITY_TYPE_EXT_SPEAKER (MEDIA_ENTITY_TYPE_EXT + 9)
> +#define MEDIA_ENTITY_TYPE_EXT_MICROPHONE (MEDIA_ENTITY_TYPE_EXT + 10)
> +#define MEDIA_ENTITY_TYPE_EXT_PLAYER (MEDIA_ENTITY_TYPE_EXT + 11)
> +#define MEDIA_ENTITY_TYPE_EXT_BROADCAST (MEDIA_ENTITY_TYPE_EXT + 12)
> +
> +#define MEDIA_ENTITY_TYPE_INT (3 << MEDIA_ENTITY_TYPE_SHIFT)
> +#define MEDIA_ENTITY_TYPE_INT_SYNTHESIZER (MEDIA_ENTITY_TYPE_INT + 1)
> +#define MEDIA_ENTITY_TYPE_INT_NOISE_SOURCE (MEDIA_ENTITY_TYPE_INT + 2)
> +#define MEDIA_ENTITY_TYPE_INT_MIXER (MEDIA_ENTITY_TYPE_INT + 3)
> +#define MEDIA_ENTITY_TYPE_INT_SELECTOR (MEDIA_ENTITY_TYPE_INT + 4)
> +#define MEDIA_ENTITY_TYPE_INT_CONTROLS (MEDIA_ENTITY_TYPE_INT + 5)
> +#define MEDIA_ENTITY_TYPE_INT_EFFECT (MEDIA_ENTITY_TYPE_INT + 6)
> +#define MEDIA_ENTITY_TYPE_INT_CHANNEL_SPLIT (MEDIA_ENTITY_TYPE_INT + 7)
> +#define MEDIA_ENTITY_TYPE_INT_CHANNEL_MERGE (MEDIA_ENTITY_TYPE_INT + 8)
>
> #define MEDIA_ENTITY_FLAG_DEFAULT (1 << 0)
>
> @@ -72,7 +92,7 @@ struct media_entity_desc {
> __u32 reserved[4];
>
> union {
> - /* Node specifications */
> + /* Device specifications */
> struct {
> __u32 major;
> __u32 minor;
> @@ -81,11 +101,15 @@ struct media_entity_desc {
> __u32 major;
> __u32 minor;
> } fb;
> - int alsa;
> + struct {
> + __u32 card;
> + __u32 device;
> + __s32 subdevice;
> + } alsa;

I will already incorporate this change, and I'll wait for other opinions on
the types before changing them.

> int dvb;
>
> /* Sub-device specifications */
> /* Nothing needed yet */
> __u8 raw[184];
> };
> };

--
Regards,

Laurent Pinchart

2010-12-14 12:40:32

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

> Hi Clemens,
>
> On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
>> I wrote:
>> > I'll see if I can draw up the ALSA-specific media stuff over the
>> weekend.
>>
>> Sorry, wrong weekend.
>>
>> Anyway, below are some remarks and a patch.
>
> Thank you. Please see my comments inline.
>
>> * Entity types
>>
>> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
>> in a graph, which does not distinguish it from other entity types
>> because all entities are part of the topology graph. I chose "device"
>> as this type describes entities that are visible as some device node to
>> other software.
>
> What this type describes is a device node. Both NODE and DEVICE can be
> confusing in my opinion, but DEVICE_NODE is a bit long.

What about DEVNODE? I think that would be a good alternative.

>> TYPE_EXT describes entities that represent some interface to the
>> external world, TYPE_INT those that are internal to the entire device.
>> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
>> to be an even more meaningless name.)
>
> SUBDEV comes from the V4L2 world, and I agree that it might not be a very
> good
> name.

SUBDEV refers to a specific type of driver. Within the v4l world it is
well defined. So I prefer to keep this. Perhaps some additional comments
or documentation can be added to clarify this.

> I'm not sure I would split entities in internal/external categories. I
> would
> create a category for connectors though.

I agree. It was always the plan to eventually add connectors, but v4l
didn't really need it (it already has an API to enumerate connectors).

>> ALSA mixer controls are not directly represented; a better fit for the
>> architecture of actual devices is that one or more mixer controls can be
>> associated with an entity. (This can be done with a field of the mixer
>> control.)
>
> Agreed.
>
>> * Entity properties
>>
>> There needs to be a mechanism to associate meta-information (properties)
>> with entities. This information should be optional and extensible, but,
>> when being handled inside the kernel, doesn't need to be more than
>> a read-only blob. I think that something like ALSA's TLV format (used
>> for mixer controls) can be used here. (I'm not mentioning the X-word
>> here, except to note that the "M" stands for "markup".)
>
> I've been thinking of adding a new ioctl for that. It's something we need
> to
> draft. The UVC driver will need it, and I'm pretty sure other V4L2 drivers
> would find it useful as well.
>
>> * Entity subtypes
>>
>> EXT_JACK_ANALOG represents any analog audio and/or video connector.
>> Properties for audio jacks would be jack type (TRS/RCA), color code,
>> line level, position, etc.
>>
>> EXT_JACK_DIGITAL represents a digital connector like S/PDIF (coax/
>> TOSLINK), ADAT, TDIF, or MADI.
>>
>> EXT_JACK_BUS represents a bus like FireWire and comes from the USB audio
>> spec. (I doubt that any devices with this entitiy will ever exist.)
>>
>> EXT_INSTRUMENT represents something like an e-guitar, keyboard, or MIDI
>> controller. (Instrument entities are typically audio sources and MIDI
>> sources and sinks, but can also be audio sinks.)
>>
>> EXT_SPEAKER also includes headphones; there might be made a case for
>> having those as a separate subtype.
>
> Shouldn't headphones be represented by an EXT_JACK_ANALOG ?
>
>> EXT_PLAYER represents a device like a CD/DVD/tape player. Recorders can
>> also write to that device, so "player" might not be an ideal name.
>>
>> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
>> cable tuners, or radios.

I don't think it is right to talk about 'represents devices'. I'd rephrase
it to 'connects to devices'.

> There's clearly an overlap with V4L here. Hopefully someone from the
> linux-
> media list can comment on this.

I don't think this will be a problem. Initially we probably won't be
enumerating connectors for V4L since it already has its own API for that.

>> INT_SYNTHESIZER converts MIDI to audio.
>>
>> INT_NOISE_SOURCE comes from the USB audio spec; this is not an attempt
>> to describe the characteristics of consumer-grade devices :-) , but
>> represents an internal noise source for level calibration or
>> measurements.
>>
>> INT_CONTROLS may have multiple independent controls (this is USB's
>> Feature Unit); INT_EFFECT may have multiple controls that affect one
>> single algorithm.
>
> I'd describe this as a feature unit/processing unit then.
>
>> INT_CHANNEL_SPLIT/MERGE are needed for HDAudio devices, whose topology
>> information has only stereo links.
>
> Some of those INT entities could also be implemented in dedicated chips,
> so I
> really think the EXT/INT split doesn't make too much sense. Should we have
> an
> AUDIO category ?
>
>> * Entity specifications
>>
>> While TYPE_DEVICE entities can be identified by their device node, other
>> entities typcially have just a numeric ID.
>
> In V4L2 sub-devices have (or rather will have once the media controller
> patches will be integrated) device nodes as well, so exposing that
> information
> is required.
>
>> For that, it would be useful to make do without separate identification
>> and
>> let the driver choose the entity ID.
>
> How would drivers do that ? What if you have two instances of the same
> chip (a
> video sensor, audio mixer, ...) on the same board ?

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by Cisco

2010-12-14 12:52:26

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Hans,

On Tuesday 14 December 2010 13:40:21 Hans Verkuil wrote:
> > On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> >> * Entity types
> >>
> >> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
> >> in a graph, which does not distinguish it from other entity types
> >> because all entities are part of the topology graph. I chose "device"
> >> as this type describes entities that are visible as some device node to
> >> other software.
> >
> > What this type describes is a device node. Both NODE and DEVICE can be
> > confusing in my opinion, but DEVICE_NODE is a bit long.
>
> What about DEVNODE? I think that would be a good alternative.

Fine with me. Clemens, any opinion on that ?

> >> TYPE_EXT describes entities that represent some interface to the
> >> external world, TYPE_INT those that are internal to the entire device.
> >> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> >> to be an even more meaningless name.)
> >
> > SUBDEV comes from the V4L2 world, and I agree that it might not be a very
> > good name.
>
> SUBDEV refers to a specific type of driver. Within the v4l world it is
> well defined. So I prefer to keep this. Perhaps some additional comments
> or documentation can be added to clarify this.

Should this be clarified by using V4L2_SUBDEV instead then ? What about ALSA
entities, should they use MEDIA_ENTITY_TYPE_ALSA_* ?

> > I'm not sure I would split entities in internal/external categories. I
> > would create a category for connectors though.
>
> I agree. It was always the plan to eventually add connectors, but v4l
> didn't really need it (it already has an API to enumerate connectors).
>
> >> ALSA mixer controls are not directly represented; a better fit for the
> >> architecture of actual devices is that one or more mixer controls can be
> >> associated with an entity. (This can be done with a field of the mixer
> >> control.)
> >
> > Agreed.
> >
> >> * Entity properties
> >>
> >> There needs to be a mechanism to associate meta-information (properties)
> >> with entities. This information should be optional and extensible, but,
> >> when being handled inside the kernel, doesn't need to be more than
> >> a read-only blob. I think that something like ALSA's TLV format (used
> >> for mixer controls) can be used here. (I'm not mentioning the X-word
> >> here, except to note that the "M" stands for "markup".)
> >
> > I've been thinking of adding a new ioctl for that. It's something we need
> > to draft. The UVC driver will need it, and I'm pretty sure other V4L2
> > drivers would find it useful as well.
> >
> >> * Entity subtypes
> >>
> >> EXT_JACK_ANALOG represents any analog audio and/or video connector.
> >> Properties for audio jacks would be jack type (TRS/RCA), color code,
> >> line level, position, etc.
> >>
> >> EXT_JACK_DIGITAL represents a digital connector like S/PDIF (coax/
> >> TOSLINK), ADAT, TDIF, or MADI.
> >>
> >> EXT_JACK_BUS represents a bus like FireWire and comes from the USB audio
> >> spec. (I doubt that any devices with this entitiy will ever exist.)
> >>
> >> EXT_INSTRUMENT represents something like an e-guitar, keyboard, or MIDI
> >> controller. (Instrument entities are typically audio sources and MIDI
> >> sources and sinks, but can also be audio sinks.)
> >>
> >> EXT_SPEAKER also includes headphones; there might be made a case for
> >> having those as a separate subtype.
> >
> > Shouldn't headphones be represented by an EXT_JACK_ANALOG ?
> >
> >> EXT_PLAYER represents a device like a CD/DVD/tape player. Recorders can
> >> also write to that device, so "player" might not be an ideal name.
> >>
> >> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
> >> cable tuners, or radios.
>
> I don't think it is right to talk about 'represents devices'. I'd rephrase
> it to 'connects to devices'.
>
> > There's clearly an overlap with V4L here. Hopefully someone from the
> > linux-media list can comment on this.
>
> I don't think this will be a problem. Initially we probably won't be
> enumerating connectors for V4L since it already has its own API for that.

My understanding is that EXT_BROADCAST really represents the TV tuners, ...,
not the connector they connect to. Some (all ?) of them are definitely V4L2
subdevs.

> >> INT_SYNTHESIZER converts MIDI to audio.
> >>
> >> INT_NOISE_SOURCE comes from the USB audio spec; this is not an attempt
> >> to describe the characteristics of consumer-grade devices :-) , but
> >> represents an internal noise source for level calibration or
> >> measurements.
> >>
> >> INT_CONTROLS may have multiple independent controls (this is USB's
> >> Feature Unit); INT_EFFECT may have multiple controls that affect one
> >> single algorithm.
> >
> > I'd describe this as a feature unit/processing unit then.
> >
> >> INT_CHANNEL_SPLIT/MERGE are needed for HDAudio devices, whose topology
> >> information has only stereo links.
> >
> > Some of those INT entities could also be implemented in dedicated chips,
> > so I really think the EXT/INT split doesn't make too much sense. Should we
> > have an AUDIO category ?
> >
> >> * Entity specifications
> >>
> >> While TYPE_DEVICE entities can be identified by their device node, other
> >> entities typcially have just a numeric ID.
> >
> > In V4L2 sub-devices have (or rather will have once the media controller
> > patches will be integrated) device nodes as well, so exposing that
> > information
> > is required.
> >
> >> For that, it would be useful to make do without separate identification
> >> and let the driver choose the entity ID.
> >
> > How would drivers do that ? What if you have two instances of the same
> > chip (a video sensor, audio mixer, ...) on the same board ?

--
Regards,

Laurent Pinchart

2010-12-14 13:32:02

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Laurent Pinchart wrote:
> On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
>> TYPE_EXT describes entities that represent some interface to the
>> external world, TYPE_INT those that are internal to the entire device.
>> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
>> to be an even more meaningless name.)
>
> SUBDEV comes from the V4L2 world, and I agree that it might not be a very good
> name.
>
> I'm not sure I would split entities in internal/external categories. I would
> create a category for connectors though.

I'm not disagreeing, but what is actually the distinction between types
and subtypes? ;-)

>> * Entity properties
>>
>> There needs to be a mechanism to associate meta-information (properties)
>> with entities. This information should be optional and extensible, but,
>> when being handled inside the kernel, doesn't need to be more than
>> a read-only blob. I think that something like ALSA's TLV format (used
>> for mixer controls) can be used here. (I'm not mentioning the X-word
>> here, except to note that the "M" stands for "markup".)
>
> I've been thinking of adding a new ioctl for that. It's something we need to
> draft. The UVC driver will need it, and I'm pretty sure other V4L2 drivers
> would find it useful as well.

I'm imagining a "read-the-properties" ioctl that just returns the
entity's blob.

>> EXT_SPEAKER also includes headphones; there might be made a case for
>> having those as a separate subtype.
>
> Shouldn't headphones be represented by an EXT_JACK_ANALOG ?

Headphone jacks are jacks; there are also USB headphones.

>> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
>> cable tuners, or radios.
>
> There's clearly an overlap with V4L here.

These come from the USB audio spec. Video devices are indeed likely to
be more detailed than just a single audio source. :)

>> INT_CONTROLS may have multiple independent controls (this is USB's
>> Feature Unit); INT_EFFECT may have multiple controls that affect one
>> single algorithm.
>
> I'd describe this as a feature unit/processing unit then.

I was aiming for more descriptive names, but I agree that the original
names might be more useful.

> Should we have an AUDIO category ?

Probably not, because there are combined audio/video jacks, any maybe
other entities.

>> * Entity specifications
>>
>> While TYPE_DEVICE entities can be identified by their device node, other
>> entities typcially have just a numeric ID.
>
> In V4L2 sub-devices have (or rather will have once the media controller
> patches will be integrated) device nodes as well, so exposing that information
> is required.

USB and HDA entities already have numeric IDs.

>> For that, it would be useful to make do without separate identification and
>> let the driver choose the entity ID.
>
> How would drivers do that ? What if you have two instances of the same chip
> (a video sensor, audio mixer, ...) on the same board ?

Then those would get different IDs; USB descriptors always describe the
entire device.

Regards,
Clemens

2010-12-14 13:49:22

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Laurent Pinchart wrote:
> On Tuesday 14 December 2010 13:40:21 Hans Verkuil wrote:
>> > On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
>> >> * Entity types
>> >>
>> >> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
>> >> in a graph, which does not distinguish it from other entity types
>> >> because all entities are part of the topology graph. I chose "device"
>> >> as this type describes entities that are visible as some device node to
>> >> other software.
>> >
>> > What this type describes is a device node. Both NODE and DEVICE can be
>> > confusing in my opinion, but DEVICE_NODE is a bit long.
>>
>> What about DEVNODE? I think that would be a good alternative.
>
> Fine with me. Clemens, any opinion on that ?

Fine with me too.

> > >> TYPE_EXT describes entities that represent some interface to the
> > >> external world, TYPE_INT those that are internal to the entire device.
> > >> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> > >> to be an even more meaningless name.)
> > >
> > > SUBDEV comes from the V4L2 world, and I agree that it might not be a very
> > > good name.
> >
> > SUBDEV refers to a specific type of driver. Within the v4l world it is
> > well defined. So I prefer to keep this. Perhaps some additional comments
> > or documentation can be added to clarify this.
>
> Should this be clarified by using V4L2_SUBDEV instead then ?

If the "SUBDEV" concept doesn't exist outside V4L, that would indeed be
better.

I don't want to rename things that come out of existing frameworks; this
naming discussion makes sense only for those entity (sub)types that can
be shared between them. Are there any, besides jacks?

> What about ALSA entities, should they use MEDIA_ENTITY_TYPE_ALSA_* ?

The entity types representing ALSA devices are already named "ALSA".

Regards,
Clemens

2010-12-14 13:54:43

by Takashi Iwai

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

At Tue, 14 Dec 2010 14:31:55 +0100,
Clemens Ladisch wrote:
>
> > Should we have an AUDIO category ?
>
> Probably not, because there are combined audio/video jacks, any maybe
> other entities.

Yes, nowadays HDMI / DP are pretty common, for example.

Takashi

2010-12-14 14:24:11

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Clemens,

On Tuesday 14 December 2010 14:31:55 Clemens Ladisch wrote:
> Laurent Pinchart wrote:
> > On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> >> TYPE_EXT describes entities that represent some interface to the
> >> external world, TYPE_INT those that are internal to the entire device.
> >> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> >> to be an even more meaningless name.)
> >
> > SUBDEV comes from the V4L2 world, and I agree that it might not be a very
> > good name.
> >
> > I'm not sure I would split entities in internal/external categories. I
> > would create a category for connectors though.
>
> I'm not disagreeing, but what is actually the distinction between types
> and subtypes? ;-)

The type is currently used to distinguish between entities that stream media
data from/to memory and other entities. They need to be handled differently in
the kernel for power management purposes for instance.

I'm not sure if we should create new types, or just remove the type/subtype
distinction and add a flag somewhere.

> >> * Entity properties
> >>
> >> There needs to be a mechanism to associate meta-information (properties)
> >> with entities. This information should be optional and extensible, but,
> >> when being handled inside the kernel, doesn't need to be more than
> >> a read-only blob. I think that something like ALSA's TLV format (used
> >> for mixer controls) can be used here. (I'm not mentioning the X-word
> >> here, except to note that the "M" stands for "markup".)
> >
> > I've been thinking of adding a new ioctl for that. It's something we need
> > to draft. The UVC driver will need it, and I'm pretty sure other V4L2
> > drivers would find it useful as well.
>
> I'm imagining a "read-the-properties" ioctl that just returns the
> entity's blob.

Martin Rubli has already proposed something similar a while ago on the linux-
media mailing list. His proposal didn't use TLV though.

> >> EXT_SPEAKER also includes headphones; there might be made a case for
> >> having those as a separate subtype.
> >
> > Shouldn't headphones be represented by an EXT_JACK_ANALOG ?
>
> Headphone jacks are jacks; there are also USB headphones.

So EXT_SPEAKER are speakers not connected through a jack (USB, internal
analog, ...) ?

> >> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
> >> cable tuners, or radios.
> >
> > There's clearly an overlap with V4L here.
>
> These come from the USB audio spec. Video devices are indeed likely to
> be more detailed than just a single audio source. :)

Does EXT_BROADCAST represent the TV tuner (or satellite receiver, cable tuner,
radio tuner, ...) itself, or the connection between the tuner and the rest of
the device ? Most TV tuner are currently handled by V4L2 and would thus turn
up as V4L2 subdevs (I'm not sure if that's what we want in the long term, but
it's at least the current situation).

> >> INT_CONTROLS may have multiple independent controls (this is USB's
> >> Feature Unit); INT_EFFECT may have multiple controls that affect one
> >> single algorithm.
> >
> > I'd describe this as a feature unit/processing unit then.
>
> I was aiming for more descriptive names, but I agree that the original
> names might be more useful.
>
> > Should we have an AUDIO category ?
>
> Probably not, because there are combined audio/video jacks, any maybe
> other entities.

> >> * Entity specifications
> >>
> >> While TYPE_DEVICE entities can be identified by their device node, other
> >> entities typcially have just a numeric ID.
> >
> > In V4L2 sub-devices have (or rather will have once the media controller
> > patches will be integrated) device nodes as well, so exposing that
> > information is required.
>
> USB and HDA entities already have numeric IDs.

Right. Same for USB Video Class.

We could let drivers set the entity ID, and have the core fill it if the value
is 0. Non-zero values would have to be checked for uniqueness though. Hans, a
comment on that ?

> >> For that, it would be useful to make do without separate identification
> >> and let the driver choose the entity ID.
> >
> > How would drivers do that ? What if you have two instances of the same
> > chip (a video sensor, audio mixer, ...) on the same board ?
>
> Then those would get different IDs; USB descriptors always describe the
> entire device.

--
Regards,

Laurent Pinchart

2010-12-14 14:49:24

by Sakari Ailus

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Clemens, Laurent, Hans & others!

Clemens Ladisch wrote:
> I wrote:
>> I'll see if I can draw up the ALSA-specific media stuff over the weekend.
>
> Sorry, wrong weekend.
>
> Anyway, below are some remarks and a patch.
>
>
> * Entity types
>
> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
> in a graph, which does not distinguish it from other entity types
> because all entities are part of the topology graph. I chose "device"
> as this type describes entities that are visible as some device node to
> other software.
>
> TYPE_EXT describes entities that represent some interface to the
> external world, TYPE_INT those that are internal to the entire device.
> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> to be an even more meaningless name.)
>
>
> ALSA mixer controls are not directly represented; a better fit for the
> architecture of actual devices is that one or more mixer controls can be
> associated with an entity. (This can be done with a field of the mixer
> control.)
>
>
> * Entity properties
>
> There needs to be a mechanism to associate meta-information (properties)
> with entities. This information should be optional and extensible, but,
> when being handled inside the kernel, doesn't need to be more than
> a read-only blob. I think that something like ALSA's TLV format (used
> for mixer controls) can be used here. (I'm not mentioning the X-word
> here, except to note that the "M" stands for "markup".)
>
>
> * Entity subtypes
>
> EXT_JACK_ANALOG represents any analog audio and/or video connector.
> Properties for audio jacks would be jack type (TRS/RCA), color code,
> line level, position, etc.
>
> EXT_JACK_DIGITAL represents a digital connector like S/PDIF (coax/
> TOSLINK), ADAT, TDIF, or MADI.
>
> EXT_JACK_BUS represents a bus like FireWire and comes from the USB audio
> spec. (I doubt that any devices with this entitiy will ever exist.)
>
> EXT_INSTRUMENT represents something like an e-guitar, keyboard, or MIDI
> controller. (Instrument entities are typically audio sources and MIDI
> sources and sinks, but can also be audio sinks.)
>
> EXT_SPEAKER also includes headphones; there might be made a case for
> having those as a separate subtype.
>
> EXT_PLAYER represents a device like a CD/DVD/tape player. Recorders can
> also write to that device, so "player" might not be an ideal name.
>
> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
> cable tuners, or radios.
>
> INT_SYNTHESIZER converts MIDI to audio.
>
> INT_NOISE_SOURCE comes from the USB audio spec; this is not an attempt
> to describe the characteristics of consumer-grade devices :-) , but
> represents an internal noise source for level calibration or measurements.
>
> INT_CONTROLS may have multiple independent controls (this is USB's
> Feature Unit); INT_EFFECT may have multiple controls that affect one
> single algorithm.
>
> INT_CHANNEL_SPLIT/MERGE are needed for HDAudio devices, whose topology
> information has only stereo links.

This naming already has been commented, but what do you think: should
the type explicitly tell what kind of interface, if any, is exported to
user space?

Only MEDIA_ENTITY_NODE_* types do this currently, and
MEDIA_ENTITY_TYPE_SUBDEV_* to some extent, but the way is not consistent
at the moment. MEDIA_ENTITY_NODE_* range has lost of different
interfaces whereas MEDIA_ENTITY_TYPE_SUBDEV_* are basically offering
v4l2_subdev and beyond that, suggesting what kind of controls might be
found from the nodes.

I would expect that the interfaces offered by the character devices
would be somewhat standardised in the end like v4l2_subdev user space
interface.

The types above are mostly describing the role of an entity, which might
be interesting as well.

Regards,

--
Sakari Ailus
[email protected]

2010-12-14 14:51:17

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

> Laurent Pinchart wrote:
>> On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
>>> TYPE_EXT describes entities that represent some interface to the
>>> external world, TYPE_INT those that are internal to the entire device.
>>> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
>>> to be an even more meaningless name.)
>>
>> SUBDEV comes from the V4L2 world, and I agree that it might not be a
>> very good
>> name.
>>
>> I'm not sure I would split entities in internal/external categories. I
>> would
>> create a category for connectors though.
>
> I'm not disagreeing, but what is actually the distinction between types
> and subtypes? ;-)

The type tells what the behavior is of an entity. E.g., type DEVNODE
represents device node(s) in userspace, V4L2_SUBDEV represents a v4l2
sub-device, etc. The subtype tells whether a V4L2_SUBDEV is a sensor or a
receiver or whatever. Nice to know, but it doesn't change the way
sub-devices work.

In the case of connectors you would create a CONNECTOR type and have a
bunch of subtypes for all the variations of connectors.

That said, I'm not sure whether the distinction is useful for DEVNODEs.
You do need to know the subtype in order to interpret the union correctly.

Laurent, does the MC code test against the DEVNODE type? I.e., does the MC
code ignore the subtype of a DEVNODE, or does it always use it?

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by Cisco

2010-12-14 14:56:49

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Hans,

On Tuesday 14 December 2010 15:51:08 Hans Verkuil wrote:
> > Laurent Pinchart wrote:
> >> On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> >>> TYPE_EXT describes entities that represent some interface to the
> >>> external world, TYPE_INT those that are internal to the entire device.
> >>> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> >>> to be an even more meaningless name.)
> >>
> >> SUBDEV comes from the V4L2 world, and I agree that it might not be a
> >> very good
> >> name.
> >>
> >> I'm not sure I would split entities in internal/external categories. I
> >> would
> >> create a category for connectors though.
> >
> > I'm not disagreeing, but what is actually the distinction between types
> > and subtypes? ;-)
>
> The type tells what the behavior is of an entity. E.g., type DEVNODE
> represents device node(s) in userspace, V4L2_SUBDEV represents a v4l2
> sub-device, etc. The subtype tells whether a V4L2_SUBDEV is a sensor or a
> receiver or whatever. Nice to know, but it doesn't change the way
> sub-devices work.
>
> In the case of connectors you would create a CONNECTOR type and have a
> bunch of subtypes for all the variations of connectors.
>
> That said, I'm not sure whether the distinction is useful for DEVNODEs.
> You do need to know the subtype in order to interpret the union correctly.
>
> Laurent, does the MC code test against the DEVNODE type? I.e., does the MC
> code ignore the subtype of a DEVNODE, or does it always use it?

The MC code uses the DEVNODE type, ignoring the subtype, for power management.
When a device node is opened all entities in the chain need to be powered up.

--
Regards,

Laurent Pinchart

2010-12-14 15:30:45

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Laurent Pinchart wrote:
> On Tuesday 14 December 2010 14:31:55 Clemens Ladisch wrote:
> > Laurent Pinchart wrote:
> > > On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> > >> EXT_SPEAKER also includes headphones; there might be made a case for
> > >> having those as a separate subtype.
> > >
> > > Shouldn't headphones be represented by an EXT_JACK_ANALOG ?
> >
> > Headphone jacks are jacks; there are also USB headphones.
>
> So EXT_SPEAKER are speakers not connected through a jack (USB, internal
> analog, ...) ?

Yes.

When there is jack, the driver often does not know what is connected.

> > >> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
> > >> cable tuners, or radios.
> > >
> > > There's clearly an overlap with V4L here.
> >
> > These come from the USB audio spec. Video devices are indeed likely to
> > be more detailed than just a single audio source. :)
>
> Does EXT_BROADCAST represent the TV tuner (or satellite receiver, cable tuner,
> radio tuner, ...) itself, or the connection between the tuner and the rest of
> the device ? Most TV tuner are currently handled by V4L2 and would thus turn
> up as V4L2 subdevs (I'm not sure if that's what we want in the long term, but
> it's at least the current situation).

>From the point of view of an audio device, this would be just some audio
source, much like a connector. We don't need this if there is some
better V4L entitity that the USB audio entity can be mapped to.

Regards,
Clemens

2010-12-14 23:49:51

[permalink] [raw]

Subject: Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links

Hi Clemens,

On Tuesday 14 December 2010 14:49:15 Clemens Ladisch wrote:
> Laurent Pinchart wrote:
> > On Tuesday 14 December 2010 13:40:21 Hans Verkuil wrote:
> >> > On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> >> >> * Entity types
> >> >>
> >> >> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a
> >> >> node in a graph, which does not distinguish it from other entity
> >> >> types because all entities are part of the topology graph. I chose
> >> >> "device" as this type describes entities that are visible as some
> >> >> device node to other software.
> >> >
> >> > What this type describes is a device node. Both NODE and DEVICE can be
> >> > confusing in my opinion, but DEVICE_NODE is a bit long.
> >>
> >> What about DEVNODE? I think that would be a good alternative.
> >
> > Fine with me. Clemens, any opinion on that ?
>
> Fine with me too.

OK I'll use that name.

> > > >> TYPE_EXT describes entities that represent some interface to the
> > > >> external world, TYPE_INT those that are internal to the entire
> > > >> device. (I'm not sure if that distinction is very useful, but
> > > >> TYPE_SUBDEV seems to be an even more meaningless name.)
> > > >
> > > > SUBDEV comes from the V4L2 world, and I agree that it might not be a
> > > > very good name.
> > >
> > > SUBDEV refers to a specific type of driver. Within the v4l world it is
> > > well defined. So I prefer to keep this. Perhaps some additional
> > > comments or documentation can be added to clarify this.
> >
> > Should this be clarified by using V4L2_SUBDEV instead then ?
>
> If the "SUBDEV" concept doesn't exist outside V4L, that would indeed be
> better.
>
> I don't want to rename things that come out of existing frameworks; this
> naming discussion makes sense only for those entity (sub)types that can
> be shared between them. Are there any, besides jacks?

Some entities like TV tuners play a dual audio/video role. I'm not sure how to
handle them, I lack experience in that field.

> > What about ALSA entities, should they use MEDIA_ENTITY_TYPE_ALSA_* ?
>
> The entity types representing ALSA devices are already named "ALSA".

I was talking about the INT_* types. They're ALSA-specific, but have no ALSA
in the type name.

--
Regards,

Laurent Pinchart

2010-12-21 16:49:56