Re: [RFC/PATCH v3 03/10] media: Entities, pads and links
Hi Sakari,
On Friday 30 July 2010 16:00:31 Sakari Ailus wrote:
Laurent Pinchart wrote:
[snip]
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
new file mode 100644
index 000..37a25bf
--- /dev/null
+++ b/include/media/media-entity.h
@@ -0,0 +1,85 @@
+#ifndef _MEDIA_ENTITY_H
+#define _MEDIA_ENTITY_H
+
+#include linux/list.h
+
+#define MEDIA_ENTITY_TYPE_NODE (1 16)
About that 16 there, could that be replaced by a #define instead? Like
MEDIA_ENTITY_TYPE_SHIFT? (I don't think we'd need
MEDIA_ENTITY_SUBTYPE_SHIFT.)
Agreed.
+#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA(MEDIA_ENTITY_TYPE_NODE
+ 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV (2 16)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER
(MEDIA_ENTITY_TYPE_SUBDEV +
1) +#define
MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER(MEDIA_ENTITY_TYPE_SUBDEV + 2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_MISC
(MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_LINK_FLAG_ACTIVE (1 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE (1 1)
+
+#define MEDIA_PAD_FLAG_INPUT (1 0)
+#define MEDIA_PAD_FLAG_OUTPUT (1 1)
+
+struct media_link {
+ struct media_pad *source; /* Source pad */
+ struct media_pad *sink; /* Sink pad */
+ struct media_link *other; /* Link in the reverse direction */
+ unsigned long flags;/* Link flags (MEDIA_LINK_FLAG_*) */
+};
+
+struct media_pad {
+ struct media_entity *entity;/* Entity this pad belongs to */
+ u16 index; /* Pad index in the entity pads array */
+ unsigned long flags;/* Pad flags (MEDIA_PAD_FLAG_*) */
+};
+
+struct media_entity {
+ struct list_head list;
+ struct media_device *parent;/* Media device this entity belongs to*/
+ u32 id; /* Entity ID, unique in the parent media
+* device context */
+ const char *name; /* Entity name */
+ u32 type; /* Entity type (MEDIA_ENTITY_TYPE_*) */
+
+ u8 num_pads;/* Number of input and output pads */
Hans suggested u16 for pads. This is a kernel structure but still it'd
be good to keep them the same IMO, even if that u16 was there for the
future.
u8 is used on some function arguments as well for these purposes.
Will fix.
+ u8 num_links; /* Number of existing links, both active
+* and inactive */
+ u8 num_backlinks; /* Number of backlinks */
+ u8 max_links; /* Maximum number of links */
Same for these.
Agreed. The number of links is expected to be = number of pads. I'll change
num_links, num_backlinks and max_links to u16 as well.
+ struct media_pad *pads; /* Pads array (num_pads elements) */
+ struct media_link *links; /* Links array (max_links elements)*/
+
+ union {
+ /* Node specifications */
+ struct {
+ u32 major;
+ u32 minor;
How about dev_t here?
dev_t doesn't seem to be a public kernel type.
+ } v4l;
+ struct {
+ u32 major;
+ u32 minor;
And here.
+ } fb;
+ int alsa;
+ int dvb;
+
+ /* Sub-device specifications */
+ /* Nothing needed yet */
+ };
+};
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v3 03/10] media: Entities, pads and links
Hi Hans,
On Sunday 01 August 2010 13:32:50 Hans Verkuil wrote:
On Thursday 29 July 2010 18:06:36 Laurent Pinchart wrote:
snip
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
new file mode 100644
index 000..37a25bf
--- /dev/null
+++ b/include/media/media-entity.h
@@ -0,0 +1,85 @@
+#ifndef _MEDIA_ENTITY_H
+#define _MEDIA_ENTITY_H
+
+#include linux/list.h
+
+#define MEDIA_ENTITY_TYPE_NODE (1 16)
I agree with Sakari that '16' should be a define.
Will do.
+#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB (MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA(MEDIA_ENTITY_TYPE_NODE
+ 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV (2 16)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER
(MEDIA_ENTITY_TYPE_SUBDEV +
1)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER
(MEDIA_ENTITY_TYPE_SUBDEV +
2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_MISC
(MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_LINK_FLAG_ACTIVE (1 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE (1 1)
+
+#define MEDIA_PAD_FLAG_INPUT (1 0)
+#define MEDIA_PAD_FLAG_OUTPUT (1 1)
+
+struct media_link {
+ struct media_pad *source; /* Source pad */
+ struct media_pad *sink; /* Sink pad */
+ struct media_link *other; /* Link in the reverse direction */
Why not rename 'other' to 'back' or 'backlink'? 'other' is not a good name
IMHO.
For forward links other is the backlink, but for backlinks other is the
forward link. I'll rename it to reverse.
+ unsigned long flags;/* Link flags (MEDIA_LINK_FLAG_*) */
+};
+
+struct media_pad {
+ struct media_entity *entity;/* Entity this pad belongs to */
+ u16 index; /* Pad index in the entity pads array */
+ unsigned long flags;/* Pad flags (MEDIA_PAD_FLAG_*) */
+};
+
+struct media_entity {
+ struct list_head list;
+ struct media_device *parent;/* Media device this entity belongs to*/
+ u32 id; /* Entity ID, unique in the parent media
+* device context */
+ const char *name; /* Entity name */
+ u32 type; /* Entity type (MEDIA_ENTITY_TYPE_*) */
+
+ u8 num_pads;/* Number of input and output pads */
+ u8 num_links; /* Number of existing links, both active
+* and inactive */
+ u8 num_backlinks; /* Number of backlinks */
+ u8 max_links; /* Maximum number of links */
+
+ struct media_pad *pads; /* Pads array (num_pads elements) */
+ struct media_link *links; /* Links array (max_links elements)*/
+
+ union {
+ /* Node specifications */
+ struct {
+ u32 major;
+ u32 minor;
+ } v4l;
+ struct {
+ u32 major;
+ u32 minor;
+ } fb;
+ int alsa;
+ int dvb;
+
+ /* Sub-device specifications */
+ /* Nothing needed yet */
+ };
+};
+
+#define MEDIA_ENTITY_TYPE_MASK 0x
+#define MEDIA_ENTITY_SUBTYPE_MASK 0x
+
+#define media_entity_type(entity) \
+ ((entity)-type MEDIA_ENTITY_TYPE_MASK)
+#define media_entity_subtype(entity) \
+ ((entity)-type MEDIA_ENTITY_SUBTYPE_MASK)
Make these inline functions to ensure correct typechecking.
OK.
Since bit 31 of the entity ID is reserved for MEDIA_ENTITY_ID_FLAG_NEXT you
probably should change the TYPE_MASK to 0x7fff. Actually, I would
change the type mask to 0x00ff, that way all top 8 bits are available
for the future.
Agreed.
+
+int media_entity_init(struct media_entity *entity, u8 num_pads,
+ struct media_pad *pads, u8 extra_links);
+void media_entity_cleanup(struct media_entity *entity);
+int media_entity_create_link(struct media_entity *source, u8 source_pad,
+ struct media_entity *sink, u8 sink_pad, u32 flags);
+
+#endif
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v3 03/10] media: Entities, pads and links
Hi Laurent,
And thanks for the patch!
Laurent Pinchart wrote:
...
diff --git a/include/media/media-device.h b/include/media/media-device.h
index bd559b0..ac96847 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -23,8 +23,10 @@
#include linux/device.h
#include linux/list.h
+#include linux/spinlock.h
#include media/media-devnode.h
+#include media/media-entity.h
/* Each instance of a media device should create the media_device struct,
* either stand-alone or embedded in a larger struct.
@@ -40,9 +42,23 @@ struct media_device {
*/
struct device *dev;
struct media_devnode devnode;
+
+ u32 entity_id;
+ struct list_head entities;
+
+ /* Protects the entities list */
+ spinlock_t lock;
};
int __must_check media_device_register(struct media_device *mdev);
void media_device_unregister(struct media_device *mdev);
+int __must_check media_device_register_entity(struct media_device *mdev,
+ struct media_entity *entity);
+void media_device_unregister_entity(struct media_entity *entity);
+
+/* Iterate over all entities. */
+#define media_device_for_each_entity(entity, mdev) \
+ list_for_each_entry(entity, (mdev)-entities, list)
+
#endif
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
new file mode 100644
index 000..37a25bf
--- /dev/null
+++ b/include/media/media-entity.h
@@ -0,0 +1,85 @@
+#ifndef _MEDIA_ENTITY_H
+#define _MEDIA_ENTITY_H
+
+#include linux/list.h
+
+#define MEDIA_ENTITY_TYPE_NODE (1 16)
About that 16 there, could that be replaced by a #define instead? Like
MEDIA_ENTITY_TYPE_SHIFT? (I don't think we'd need
MEDIA_ENTITY_SUBTYPE_SHIFT.)
+#define MEDIA_ENTITY_TYPE_NODE_V4L (MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB(MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA (MEDIA_ENTITY_TYPE_NODE + 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB (MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV (2 16)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER (MEDIA_ENTITY_TYPE_SUBDEV + 1)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER (MEDIA_ENTITY_TYPE_SUBDEV + 2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_MISC
(MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_LINK_FLAG_ACTIVE (1 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE(1 1)
+
+#define MEDIA_PAD_FLAG_INPUT (1 0)
+#define MEDIA_PAD_FLAG_OUTPUT(1 1)
+
+struct media_link {
+ struct media_pad *source; /* Source pad */
+ struct media_pad *sink; /* Sink pad */
+ struct media_link *other; /* Link in the reverse direction */
+ unsigned long flags;/* Link flags (MEDIA_LINK_FLAG_*) */
+};
+
+struct media_pad {
+ struct media_entity *entity;/* Entity this pad belongs to */
+ u16 index; /* Pad index in the entity pads array */
+ unsigned long flags;/* Pad flags (MEDIA_PAD_FLAG_*) */
+};
+
+struct media_entity {
+ struct list_head list;
+ struct media_device *parent;/* Media device this entity belongs to*/
+ u32 id; /* Entity ID, unique in the parent media
+ * device context */
+ const char *name; /* Entity name */
+ u32 type; /* Entity type (MEDIA_ENTITY_TYPE_*) */
+
+ u8 num_pads;/* Number of input and output pads */
Hans suggested u16 for pads. This is a kernel structure but still it'd
be good to keep them the same IMO, even if that u16 was there for the
future.
u8 is used on some function arguments as well for these purposes.
+ u8 num_links; /* Number of existing links, both active
+ * and inactive */
+ u8 num_backlinks; /* Number of backlinks */
+ u8 max_links; /* Maximum number of links */
Same for these.
+ struct media_pad *pads; /* Pads array (num_pads elements) */
+ struct media_link *links; /* Links array (max_links elements)*/
+
+ union {
+ /* Node specifications */
+ struct {
+ u32 major;
+ u32 minor;
How about dev_t here?
+ } v4l;
+ struct {
+ u32 major;
+ u32 minor;
And here.
+ } fb;
+ int alsa;
+ int dvb;
+
+ /* Sub-device specifications */
+ /* Nothing needed yet */
+ };
+};
+
+#define MEDIA_ENTITY_TYPE_MASK 0x
+#define MEDIA_ENTITY_SUBTYPE_MASK0x
+
+#define media_entity_type(entity) \
+
[RFC/PATCH v3 03/10] 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 hverk...@xs4all.nl initial work. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com --- Documentation/media-framework.txt | 130 + drivers/media/Makefile|2 +- drivers/media/media-device.c | 53 ++ drivers/media/media-entity.c | 144 + include/media/media-device.h | 16 include/media/media-entity.h | 85 ++ 6 files changed, 429 insertions(+), 1 deletions(-) create mode 100644 drivers/media/media-entity.c create mode 100644 include/media/media-entity.h diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt index b942c8f..5bd7216 100644 --- a/Documentation/media-framework.txt +++ b/Documentation/media-framework.txt @@ -35,6 +35,30 @@ belong to userspace. The media kernel API aims at solving those problems. +Abstract media device model +--- + +Discovering a device internal topology, and configuring it at runtime, is one +of the goals of the media framework. To achieve this, hardware devices are +modeled as an oriented graph of building blocks called entities connected +through pads. + +An entity is a basic media hardware building block. It can correspond to +a large variety of logical blocks such as physical hardware devices +(CMOS sensor for instance), logical hardware devices (a building block +in a System-on-Chip image processing pipeline), DMA channels or physical +connectors. + +A pad is a connection endpoint through which an entity can interact with +other entities. Data (not restricted to video) produced by an entity +flows from the entity's output to one or more entity inputs. Pads should +not be confused with physical pins at chip boundaries. + +A link is a point-to-point oriented connection between two pads, either +on the same entity or on different entities. Data flows from a source +pad to a sink pad. + + Media device @@ -66,3 +90,109 @@ Drivers unregister media device instances by calling Unregistering a media device that hasn't been registered is *NOT* safe. + +Entities, pads and links + + +- Entities + +Entities are represented by a struct media_entity instance, defined in +include/media/media-entity.h. The structure is usually embedded into a +higher-level structure, such as a v4l2_subdev or video_device instance, +although drivers can allocate entities directly. + +Drivers initialize entities by calling + + media_entity_init(struct media_entity *entity, u8 num_pads, + struct media_pad *pads, u8 extra_links); + +The media_entity name and type fields can be initialized before or after +calling media_entity_init. Entities embedded in higher-level standard +structures have those fields set by the higher-level framework. + +As
