* [PATCH RFC v5 0/8] MC preparation patches
@ 2015-08-18 20:04 Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 1/8] [media] media: create a macro to get entity ID Mauro Carvalho Chehab
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
Those are the initial patches from my RFCv3 series of MC changes,
plus one additional renaming patch and two debug ones. They address
the comments received on PATCH v4 series.
The first 5 patches on this series ensures that all existing object
types (entities, pads and links) will have an unique ID, as agreed
at the MC workshop.
The next two patches add two debug functions, that helps with the
tests of the MC changes. Both are enabled only if DEBUG or dynamic
debug is enabled.
The first one just help to identify when the media_device register,
remove and unregister functions are called. It helps to identify if
those events happen before or after object creation.
The second one is more interesting: it hooks at the object init and
remove functions and dump what's there at the new object when it
got created. This is very useful to test the future patches, as we'll
be able to track any topology changes.
Also, it demonstates the capability of the functions
media_gobj_init() and media_gobj_remove() to track topology
changes. Tracking topology changes is fundamental for the new API,
in order to implement G_TOPOLOGY ioctl. They should contain, in
the future, a callback to warn the several drivers envolved at the
MC topology build about topology changes.
The last patch on this preparation series is just a renaming patch,
that will avoid mess when future patches introduce the
entity->interface links.
They're tested using the cx231xx V4L2 support.
Those are the new debug messages when the device is probed:
[ 2684.265619] cx231xx 3-2.4:1.1: Media device registered
[ 2684.357670] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000001 entity#1 'cx25840 19-0044'
[ 2684.359191] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000001 cx25840 19-0044 pad#1
[ 2684.360693] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000002 cx25840 19-0044 pad#2
[ 2684.362208] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000003 cx25840 19-0044 pad#3
[ 2686.437870] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000002 entity#2 '(tuner unset)'
[ 2686.439474] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000004 (tuner unset) pad#4
[ 2688.450710] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000003 entity#3 'cx231xx #0 video'
[ 2688.452395] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000005 cx231xx #0 video pad#5
[ 2688.455892] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000004 entity#4 'cx231xx #0 vbi'
[ 2688.457632] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000006 cx231xx #0 vbi pad#6
[ 2688.500233] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000001 link#1: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
[ 2688.501830] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000002 link#2: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
[ 2688.503415] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000003 link#3: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
[ 2688.504977] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000004 link#4: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
[ 2688.506505] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000005 link#5: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
[ 2688.508074] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000006 link#6: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
Those are the new debug messages when the device is removed:
[ 2953.872780] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000002 link#2: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
[ 2953.874470] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000003 link#3: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
[ 2953.876199] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000005 link#5: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
[ 2953.877830] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000001 cx25840 19-0044 pad#1
[ 2953.879454] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000002 cx25840 19-0044 pad#2
[ 2953.881020] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000003 cx25840 19-0044 pad#3
[ 2953.882569] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000001 entity#1 'cx25840 19-0044'
[ 2953.884093] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000001 link#1: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
[ 2953.885611] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000004 NXP TDA18271HD pad#4
[ 2953.887200] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000002 entity#2 'NXP TDA18271HD'
[ 2953.888638] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000004 link#4: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
[ 2953.890093] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000005 cx231xx #0 video pad#5
[ 2953.891549] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000003 entity#3 'cx231xx #0 video'
[ 2953.893006] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000006 link#6: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
[ 2953.894471] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000006 cx231xx #0 vbi pad#6
[ 2953.895955] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000004 entity#4 'cx231xx #0 vbi'
[ 2953.897505] cx231xx 3-2.4:1.1: Media device released
[ 2953.898924] cx231xx 3-2.4:1.1: Media device unregistered
It should be noticed that both links and backlinks are reported
on the above. That's because now they're implemented as two separate
links right now.
Mauro Carvalho Chehab (8):
[media] media: create a macro to get entity ID
[media] media: add a common struct to be embed on media graph objects
[media] media: use media_gobj inside entities
[media] media: use media_gobj inside pads
[media] media: use media_gobj inside links
[media] media: add messages when media device gets (un)registered
[media] media: add a debug message to warn about gobj creation/removal
[media] media: rename the function that create pad links
Documentation/media-framework.txt | 2 +-
drivers/media/dvb-core/dvbdev.c | 8 +-
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 4 +-
drivers/media/i2c/s5k5baf.c | 2 +-
drivers/media/i2c/smiapp/smiapp-core.c | 4 +-
drivers/media/media-device.c | 41 +++++--
drivers/media/media-entity.c | 123 ++++++++++++++++++++-
drivers/media/platform/exynos4-is/media-dev.c | 16 +--
drivers/media/platform/omap3isp/isp.c | 18 +--
drivers/media/platform/omap3isp/ispccdc.c | 2 +-
drivers/media/platform/omap3isp/ispccp2.c | 2 +-
drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
drivers/media/platform/omap3isp/isppreview.c | 4 +-
drivers/media/platform/omap3isp/ispresizer.c | 4 +-
drivers/media/platform/s3c-camif/camif-core.c | 4 +-
drivers/media/platform/vsp1/vsp1_drv.c | 4 +-
drivers/media/platform/vsp1/vsp1_rpf.c | 2 +-
drivers/media/platform/vsp1/vsp1_video.c | 4 +-
drivers/media/platform/vsp1/vsp1_wpf.c | 2 +-
drivers/media/platform/xilinx/xilinx-vipp.c | 4 +-
drivers/media/usb/cx231xx/cx231xx-cards.c | 6 +-
drivers/media/usb/uvc/uvc_entity.c | 2 +-
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 2 +-
drivers/staging/media/davinci_vpfe/dm365_isif.c | 2 +-
drivers/staging/media/davinci_vpfe/dm365_resizer.c | 8 +-
.../staging/media/davinci_vpfe/vpfe_mc_capture.c | 10 +-
drivers/staging/media/omap4iss/iss.c | 12 +-
drivers/staging/media/omap4iss/iss_csi2.c | 2 +-
drivers/staging/media/omap4iss/iss_ipipeif.c | 2 +-
drivers/staging/media/omap4iss/iss_resizer.c | 2 +-
include/media/media-device.h | 7 +-
include/media/media-entity.h | 82 +++++++++++++-
32 files changed, 302 insertions(+), 87 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH RFC v5 1/8] [media] media: create a macro to get entity ID
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects Mauro Carvalho Chehab
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Laurent Pinchart, linux-sh
Instead of accessing directly entity.id, let's create a macro,
as this field will be moved into a common struct later on.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 7b39440192d6..56aebe12aed8 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -75,8 +75,8 @@ static struct media_entity *find_entity(struct media_device *mdev, u32 id)
spin_lock(&mdev->lock);
media_device_for_each_entity(entity, mdev) {
- if ((entity->id == id && !next) ||
- (entity->id > id && next)) {
+ if (((media_entity_id(entity) == id) && !next) ||
+ ((media_entity_id(entity) > id) && next)) {
spin_unlock(&mdev->lock);
return entity;
}
@@ -102,7 +102,7 @@ static long media_device_enum_entities(struct media_device *mdev,
if (ent == NULL)
return -EINVAL;
- u_ent.id = ent->id;
+ u_ent.id = media_entity_id(ent);
if (ent->name)
strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
u_ent.type = ent->type;
@@ -120,7 +120,7 @@ static long media_device_enum_entities(struct media_device *mdev,
static void media_device_kpad_to_upad(const struct media_pad *kpad,
struct media_pad_desc *upad)
{
- upad->entity = kpad->entity->id;
+ upad->entity = media_entity_id(kpad->entity);
upad->index = kpad->index;
upad->flags = kpad->flags;
}
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 949e5f92cbdc..cb0ac4e0dfa5 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -140,10 +140,10 @@ void media_entity_graph_walk_start(struct media_entity_graph *graph,
graph->stack[graph->top].entity = NULL;
bitmap_zero(graph->entities, MEDIA_ENTITY_ENUM_MAX_ID);
- if (WARN_ON(entity->id >= MEDIA_ENTITY_ENUM_MAX_ID))
+ if (WARN_ON(media_entity_id(entity) >= MEDIA_ENTITY_ENUM_MAX_ID))
return;
- __set_bit(entity->id, graph->entities);
+ __set_bit(media_entity_id(entity), graph->entities);
stack_push(graph, entity);
}
EXPORT_SYMBOL_GPL(media_entity_graph_walk_start);
@@ -184,11 +184,11 @@ media_entity_graph_walk_next(struct media_entity_graph *graph)
/* Get the entity in the other end of the link . */
next = media_entity_other(entity, link);
- if (WARN_ON(next->id >= MEDIA_ENTITY_ENUM_MAX_ID))
+ if (WARN_ON(media_entity_id(next) >= MEDIA_ENTITY_ENUM_MAX_ID))
return NULL;
/* Has the entity already been visited? */
- if (__test_and_set_bit(next->id, graph->entities)) {
+ if (__test_and_set_bit(media_entity_id(next), graph->entities)) {
link_top(graph)++;
continue;
}
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 17f08973f835..debe4e539df6 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -352,10 +352,10 @@ static int vsp1_pipeline_validate_branch(struct vsp1_pipeline *pipe,
break;
/* Ensure the branch has no loop. */
- if (entities & (1 << entity->subdev.entity.id))
+ if (entities & (1 << media_entity_id(&entity->subdev.entity)))
return -EPIPE;
- entities |= 1 << entity->subdev.entity.id;
+ entities |= 1 << media_entity_id(&entity->subdev.entity);
/* UDS can't be chained. */
if (entity->type == VSP1_ENTITY_UDS) {
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 8b21a4d920d9..0a66fc225559 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -113,6 +113,11 @@ static inline u32 media_entity_subtype(struct media_entity *entity)
return entity->type & MEDIA_ENT_SUBTYPE_MASK;
}
+static inline u32 media_entity_id(struct media_entity *entity)
+{
+ return entity->id;
+}
+
#define MEDIA_ENTITY_ENUM_MAX_DEPTH 16
#define MEDIA_ENTITY_ENUM_MAX_ID 64
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 1/8] [media] media: create a macro to get entity ID Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-19 8:17 ` Hans Verkuil
2015-08-18 20:04 ` [PATCH RFC v5 3/8] [media] media: use media_gobj inside entities Mauro Carvalho Chehab
` (6 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
Due to the MC API proposed changes, we'll need to have an unique
object ID for all graph objects, and have some shared fields
that will be common on all media graph objects.
Right now, the only common object is the object ID, but other
fields will be added later on.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index cb0ac4e0dfa5..4834172bf6f8 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -27,6 +27,38 @@
#include <media/media-device.h>
/**
+ * media_gobj_init - Initialize a graph object
+ *
+ * @mdev: Pointer to the media_device that contains the object
+ * @type: Type of the object
+ * @gobj: Pointer to the object
+ *
+ * This routine initializes the embedded struct media_gobj inside a
+ * media graph object. It is called automatically if media_*_create()
+ * calls are used. However, if the object (entity, link, pad, interface)
+ * is embedded on some other object, this function should be called before
+ * registering the object at the media controller.
+ */
+void media_gobj_init(struct media_device *mdev,
+ enum media_gobj_type type,
+ struct media_gobj *gobj)
+{
+ /* For now, nothing to do */
+}
+
+/**
+ * media_gobj_remove - Stop using a graph object on a media device
+ *
+ * @graph_obj: Pointer to the object
+ *
+ * This should be called at media_device_unregister_*() routines
+ */
+void media_gobj_remove(struct media_gobj *gobj)
+{
+ /* For now, nothing to do */
+}
+
+/**
* media_entity_init - Initialize a media entity
*
* @num_pads: Total number of sink and source pads.
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 0a66fc225559..762593c7424f 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -28,6 +28,37 @@
#include <linux/list.h>
#include <linux/media.h>
+/* Enums used internally at the media controller to represent graphs */
+
+/**
+ * enum media_gobj_type - type of a graph element
+ *
+ */
+enum media_gobj_type {
+ /* FIXME: add the types here, as we embed media_gobj */
+ MEDIA_GRAPH_NONE
+};
+
+#define BITS_PER_TYPE 8
+#define BITS_PER_LOCAL_ID (32 - BITS_PER_TYPE)
+
+/* Structs to represent the objects that belong to a media graph */
+
+/**
+ * struct media_gobj - Define a graph object.
+ *
+ * @id: Non-zero object ID identifier. The ID should be unique
+ * inside a media_device, as it is composed by
+ * BITS_PER_TYPE to store the type plus BITS_PER_LOCAL_ID
+ * to store a per-type ID (called as "local ID").
+ *
+ * All elements on the media graph should have this struct embedded
+ */
+struct media_gobj {
+ u32 id;
+};
+
+
struct media_pipeline {
};
@@ -118,6 +149,26 @@ static inline u32 media_entity_id(struct media_entity *entity)
return entity->id;
}
+static inline enum media_gobj_type media_type(struct media_gobj *gobj)
+{
+ return gobj->id >> BITS_PER_LOCAL_ID;
+}
+
+static inline u32 media_localid(struct media_gobj *gobj)
+{
+ return gobj->id & GENMASK(BITS_PER_LOCAL_ID - 1, 0);
+}
+
+static inline u32 media_gobj_gen_id(enum media_gobj_type type, u32 local_id)
+{
+ u32 id;
+
+ id = type << BITS_PER_LOCAL_ID;
+ id |= GENMASK(BITS_PER_LOCAL_ID - 1, 0) & local_id;
+
+ return id;
+}
+
#define MEDIA_ENTITY_ENUM_MAX_DEPTH 16
#define MEDIA_ENTITY_ENUM_MAX_ID 64
@@ -131,6 +182,14 @@ struct media_entity_graph {
int top;
};
+#define gobj_to_entity(gobj) \
+ container_of(gobj, struct media_entity, graph_obj)
+
+void media_gobj_init(struct media_device *mdev,
+ enum media_gobj_type type,
+ struct media_gobj *gobj);
+void media_gobj_remove(struct media_gobj *gobj);
+
int media_entity_init(struct media_entity *entity, u16 num_pads,
struct media_pad *pads);
void media_entity_cleanup(struct media_entity *entity);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 3/8] [media] media: use media_gobj inside entities
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 1/8] [media] media: create a macro to get entity ID Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 4/8] [media] media: use media_gobj inside pads Mauro Carvalho Chehab
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
As entities are graph elements, let's embed media_gobj
on it. That ensures an unique ID for entities that can be
global along the entire media controller.
For now, we'll keep the already existing entity ID. Such
field need to be dropped at some point, but for now, let's
not do this, to avoid needing to review all drivers and
the userspace apps.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 56aebe12aed8..e6caec8f62b7 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -377,7 +377,6 @@ 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);
mutex_init(&mdev->graph_mutex);
@@ -431,10 +430,8 @@ int __must_check media_device_register_entity(struct media_device *mdev,
entity->parent = mdev;
spin_lock(&mdev->lock);
- if (entity->id == 0)
- entity->id = mdev->entity_id++;
- else
- mdev->entity_id = max(entity->id + 1, mdev->entity_id);
+ /* Initialize media_gobj embedded at the entity */
+ media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
list_add_tail(&entity->list, &mdev->entities);
spin_unlock(&mdev->lock);
@@ -457,6 +454,7 @@ void media_device_unregister_entity(struct media_entity *entity)
return;
spin_lock(&mdev->lock);
+ media_gobj_remove(&entity->graph_obj);
list_del(&entity->list);
spin_unlock(&mdev->lock);
entity->parent = NULL;
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 4834172bf6f8..888cb88e19bf 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -43,7 +43,12 @@ void media_gobj_init(struct media_device *mdev,
enum media_gobj_type type,
struct media_gobj *gobj)
{
- /* For now, nothing to do */
+ /* Create a per-type unique object ID */
+ switch (type) {
+ case MEDIA_GRAPH_ENTITY:
+ gobj->id = media_gobj_gen_id(type, ++mdev->entity_id);
+ break;
+ }
}
/**
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 6e6db78f1ee2..3b9a31c8eba9 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -41,7 +41,7 @@ struct device;
* @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
+ * @entity_id: Unique ID used on the last entity registered
* @entities: List of registered entities
* @lock: Entities list lock
* @graph_mutex: Entities graph operation lock
@@ -69,6 +69,7 @@ struct media_device {
u32 driver_version;
u32 entity_id;
+
struct list_head entities;
/* Protects the entities list */
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 762593c7424f..18d74a44091c 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -33,10 +33,10 @@
/**
* enum media_gobj_type - type of a graph element
*
+ * @MEDIA_GRAPH_ENTITY: Identify a media entity
*/
enum media_gobj_type {
- /* FIXME: add the types here, as we embed media_gobj */
- MEDIA_GRAPH_NONE
+ MEDIA_GRAPH_ENTITY,
};
#define BITS_PER_TYPE 8
@@ -92,10 +92,9 @@ struct media_entity_operations {
};
struct media_entity {
+ struct media_gobj graph_obj;
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_ENT_T_*) */
u32 revision; /* Entity revision, driver specific */
@@ -146,7 +145,7 @@ static inline u32 media_entity_subtype(struct media_entity *entity)
static inline u32 media_entity_id(struct media_entity *entity)
{
- return entity->id;
+ return entity->graph_obj.id;
}
static inline enum media_gobj_type media_type(struct media_gobj *gobj)
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 4/8] [media] media: use media_gobj inside pads
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
` (2 preceding siblings ...)
2015-08-18 20:04 ` [PATCH RFC v5 3/8] [media] media: use media_gobj inside entities Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 5/8] [media] media: use media_gobj inside links Mauro Carvalho Chehab
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
PADs also need unique object IDs that won't conflict with
the entity object IDs.
The pad objects are currently created via media_entity_init()
and, once created, never change.
While this will likely change in the future in order to
support dynamic changes, for now we'll keep PADs as arrays
and initialize the media_gobj embedded structs when
registering the entity.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index e6caec8f62b7..24551d4ddfb8 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -425,6 +425,8 @@ EXPORT_SYMBOL_GPL(media_device_unregister);
int __must_check media_device_register_entity(struct media_device *mdev,
struct media_entity *entity)
{
+ int i;
+
/* Warn if we apparently re-register an entity */
WARN_ON(entity->parent != NULL);
entity->parent = mdev;
@@ -433,6 +435,12 @@ int __must_check media_device_register_entity(struct media_device *mdev,
/* Initialize media_gobj embedded at the entity */
media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
list_add_tail(&entity->list, &mdev->entities);
+
+ /* Initialize objects at the pads */
+ for (i = 0; i < entity->num_pads; i++)
+ media_gobj_init(mdev, MEDIA_GRAPH_PAD,
+ &entity->pads[i].graph_obj);
+
spin_unlock(&mdev->lock);
return 0;
@@ -448,12 +456,15 @@ EXPORT_SYMBOL_GPL(media_device_register_entity);
*/
void media_device_unregister_entity(struct media_entity *entity)
{
+ int i;
struct media_device *mdev = entity->parent;
if (mdev == NULL)
return;
spin_lock(&mdev->lock);
+ for (i = 0; i < entity->num_pads; i++)
+ media_gobj_remove(&entity->pads[i].graph_obj);
media_gobj_remove(&entity->graph_obj);
list_del(&entity->list);
spin_unlock(&mdev->lock);
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 888cb88e19bf..377c6655c5d0 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -48,6 +48,9 @@ void media_gobj_init(struct media_device *mdev,
case MEDIA_GRAPH_ENTITY:
gobj->id = media_gobj_gen_id(type, ++mdev->entity_id);
break;
+ case MEDIA_GRAPH_PAD:
+ gobj->id = media_gobj_gen_id(type, ++mdev->pad_id);
+ break;
}
}
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 3b9a31c8eba9..4b5d1ee2b67e 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -42,6 +42,7 @@ struct device;
* @hw_revision: Hardware device revision
* @driver_version: Device driver version
* @entity_id: Unique ID used on the last entity registered
+ * @pad_id: Unique ID used on the last pad registered
* @entities: List of registered entities
* @lock: Entities list lock
* @graph_mutex: Entities graph operation lock
@@ -69,6 +70,7 @@ struct media_device {
u32 driver_version;
u32 entity_id;
+ u32 pad_id;
struct list_head entities;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 18d74a44091c..01ae1e320e36 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -34,9 +34,11 @@
* enum media_gobj_type - type of a graph element
*
* @MEDIA_GRAPH_ENTITY: Identify a media entity
+ * @MEDIA_GRAPH_PAD: Identify a media pad
*/
enum media_gobj_type {
MEDIA_GRAPH_ENTITY,
+ MEDIA_GRAPH_PAD,
};
#define BITS_PER_TYPE 8
@@ -70,6 +72,7 @@ struct media_link {
};
struct media_pad {
+ struct media_gobj graph_obj;
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_FL_*) */
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 5/8] [media] media: use media_gobj inside links
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
` (3 preceding siblings ...)
2015-08-18 20:04 ` [PATCH RFC v5 4/8] [media] media: use media_gobj inside pads Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-19 8:18 ` Hans Verkuil
2015-08-18 20:04 ` [PATCH RFC v5 6/8] [media] media: add messages when media device gets (un)registered Mauro Carvalho Chehab
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
Just like entities and pads, links also need to have unique
Object IDs along a given media controller.
So, let's add a media_gobj inside it and initialize
the object then a new link is created.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 24551d4ddfb8..7f512223249b 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -436,6 +436,13 @@ int __must_check media_device_register_entity(struct media_device *mdev,
media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
list_add_tail(&entity->list, &mdev->entities);
+ /*
+ * Initialize objects at the links
+ * in the case where links got created before entity register
+ */
+ for (i = 0; i < entity->num_links; i++)
+ media_gobj_init(mdev, MEDIA_GRAPH_LINK,
+ &entity->links[i].graph_obj);
/* Initialize objects at the pads */
for (i = 0; i < entity->num_pads; i++)
media_gobj_init(mdev, MEDIA_GRAPH_PAD,
@@ -463,6 +470,8 @@ void media_device_unregister_entity(struct media_entity *entity)
return;
spin_lock(&mdev->lock);
+ for (i = 0; i < entity->num_links; i++)
+ media_gobj_remove(&entity->links[i].graph_obj);
for (i = 0; i < entity->num_pads; i++)
media_gobj_remove(&entity->pads[i].graph_obj);
media_gobj_remove(&entity->graph_obj);
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 377c6655c5d0..a6be50e04736 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -51,6 +51,9 @@ void media_gobj_init(struct media_device *mdev,
case MEDIA_GRAPH_PAD:
gobj->id = media_gobj_gen_id(type, ++mdev->pad_id);
break;
+ case MEDIA_GRAPH_LINK:
+ gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
+ break;
}
}
@@ -469,6 +472,13 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
entity->links = links;
}
+ /* Initialize graph object embedded at the new link */
+ if (entity->parent)
+ media_gobj_init(entity->parent, MEDIA_GRAPH_LINK,
+ &entity->links[entity->num_links].graph_obj);
+ else
+ pr_warn("Link created before entity register!\n");
+
return &entity->links[entity->num_links++];
}
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 4b5d1ee2b67e..e0f63c0da2dd 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -43,6 +43,7 @@ struct device;
* @driver_version: Device driver version
* @entity_id: Unique ID used on the last entity registered
* @pad_id: Unique ID used on the last pad registered
+ * @link_id: Unique ID used on the last link registered
* @entities: List of registered entities
* @lock: Entities list lock
* @graph_mutex: Entities graph operation lock
@@ -71,6 +72,7 @@ struct media_device {
u32 entity_id;
u32 pad_id;
+ u32 link_id;
struct list_head entities;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 01ae1e320e36..2ffe015629fa 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -35,10 +35,12 @@
*
* @MEDIA_GRAPH_ENTITY: Identify a media entity
* @MEDIA_GRAPH_PAD: Identify a media pad
+ * @MEDIA_GRAPH_LINK: Identify a media link
*/
enum media_gobj_type {
MEDIA_GRAPH_ENTITY,
MEDIA_GRAPH_PAD,
+ MEDIA_GRAPH_LINK,
};
#define BITS_PER_TYPE 8
@@ -65,6 +67,7 @@ struct media_pipeline {
};
struct media_link {
+ struct media_gobj graph_obj;
struct media_pad *source; /* Source pad */
struct media_pad *sink; /* Sink pad */
struct media_link *reverse; /* Link in the reverse direction */
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 6/8] [media] media: add messages when media device gets (un)registered
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
` (4 preceding siblings ...)
2015-08-18 20:04 ` [PATCH RFC v5 5/8] [media] media: use media_gobj inside links Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal Mauro Carvalho Chehab
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
We can only free the media device after being sure that no
graph object is used.
In order to help tracking it, let's add debug messages
that will print when the media controller gets registered
or unregistered.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 7f512223249b..d72d283c4f29 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -357,6 +357,7 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
static void media_device_release(struct media_devnode *mdev)
{
+ dev_dbg(mdev->parent, "Media device released\n");
}
/**
@@ -395,6 +396,8 @@ int __must_check __media_device_register(struct media_device *mdev,
return ret;
}
+ dev_dbg(mdev->dev, "Media device registered\n");
+
return 0;
}
EXPORT_SYMBOL_GPL(__media_device_register);
@@ -414,6 +417,8 @@ void media_device_unregister(struct media_device *mdev)
device_remove_file(&mdev->devnode.dev, &dev_attr_model);
media_devnode_unregister(&mdev->devnode);
+
+ dev_dbg(mdev->dev, "Media device unregistered\n");
}
EXPORT_SYMBOL_GPL(media_device_unregister);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
` (5 preceding siblings ...)
2015-08-18 20:04 ` [PATCH RFC v5 6/8] [media] media: add messages when media device gets (un)registered Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-19 8:36 ` Hans Verkuil
2015-08-18 20:04 ` [PATCH RFC v5 8/8] [media] media: rename the function that create pad links Mauro Carvalho Chehab
2015-08-18 20:08 ` [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
8 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
It helps to check if the media controller is doing the
right thing with the object creation and removal.
No extra code/data will be produced if DEBUG or
CONFIG_DYNAMIC_DEBUG is not enabled.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index a6be50e04736..9e5ebc83ff76 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -27,6 +27,67 @@
#include <media/media-device.h>
/**
+ * dev_dbg_obj - Prints in debug mode a change on some object
+ *
+ * @event_name: Name of the event to report. Could be __func__
+ * @gobj: Pointer to the object
+ *
+ * Enabled only if DEBUG or CONFIG_DYNAMIC_DEBUG. Otherwise, it
+ * won't produce any code.
+ */
+static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
+{
+#if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
+ static const char *gobj_type[] = {
+ [MEDIA_GRAPH_ENTITY] = "entity",
+ [MEDIA_GRAPH_PAD] = "pad",
+ [MEDIA_GRAPH_LINK] = "link",
+ };
+
+ switch (media_type(gobj)) {
+ case MEDIA_GRAPH_ENTITY:
+ dev_dbg(gobj->mdev->dev,
+ "%s: id 0x%08x %s#%d '%s'\n",
+ event_name, gobj->id,
+ gobj_type[media_type(gobj)],
+ media_localid(gobj),
+ gobj_to_entity(gobj)->name);
+ break;
+ case MEDIA_GRAPH_LINK:
+ {
+ struct media_link *link = gobj_to_link(gobj);
+
+ dev_dbg(gobj->mdev->dev,
+ "%s: id 0x%08x %s#%d: '%s' %s#%d ==> '%s' %s#%d\n",
+ event_name, gobj->id,
+ gobj_type[media_type(gobj)],
+ media_localid(gobj),
+
+ link->source->entity->name,
+ gobj_type[media_type(&link->source->graph_obj)],
+ media_localid(&link->source->graph_obj),
+
+ link->sink->entity->name,
+ gobj_type[media_type(&link->sink->graph_obj)],
+ media_localid(&link->sink->graph_obj));
+ break;
+ }
+ case MEDIA_GRAPH_PAD:
+ {
+ struct media_pad *pad = gobj_to_pad(gobj);
+
+ dev_dbg(gobj->mdev->dev,
+ "%s: id 0x%08x %s %s#%d\n",
+ event_name, gobj->id,
+ pad->entity->name,
+ gobj_type[media_type(gobj)],
+ media_localid(gobj));
+ }
+ }
+#endif
+}
+
+/**
* media_gobj_init - Initialize a graph object
*
* @mdev: Pointer to the media_device that contains the object
@@ -43,6 +104,8 @@ void media_gobj_init(struct media_device *mdev,
enum media_gobj_type type,
struct media_gobj *gobj)
{
+ gobj->mdev = mdev;
+
/* Create a per-type unique object ID */
switch (type) {
case MEDIA_GRAPH_ENTITY:
@@ -55,6 +118,7 @@ void media_gobj_init(struct media_device *mdev,
gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
break;
}
+ dev_dbg_obj(__func__, gobj);
}
/**
@@ -66,7 +130,7 @@ void media_gobj_init(struct media_device *mdev,
*/
void media_gobj_remove(struct media_gobj *gobj)
{
- /* For now, nothing to do */
+ dev_dbg_obj(__func__, gobj);
}
/**
@@ -472,13 +536,6 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
entity->links = links;
}
- /* Initialize graph object embedded at the new link */
- if (entity->parent)
- media_gobj_init(entity->parent, MEDIA_GRAPH_LINK,
- &entity->links[entity->num_links].graph_obj);
- else
- pr_warn("Link created before entity register!\n");
-
return &entity->links[entity->num_links++];
}
@@ -501,6 +558,8 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
link->sink = &sink->pads[sink_pad];
link->flags = flags;
+ media_gobj_init(source->parent, MEDIA_GRAPH_LINK, &link->graph_obj);
+
/* Create the backlink. Backlinks are used to help graph traversal and
* are not reported to userspace.
*/
@@ -514,6 +573,8 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
backlink->sink = &sink->pads[sink_pad];
backlink->flags = flags;
+ media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
+
link->reverse = backlink;
backlink->reverse = link;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 2ffe015629fa..db1f54f09723 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -59,6 +59,7 @@ enum media_gobj_type {
* All elements on the media graph should have this struct embedded
*/
struct media_gobj {
+ struct media_device *mdev;
u32 id;
};
@@ -190,6 +191,12 @@ struct media_entity_graph {
#define gobj_to_entity(gobj) \
container_of(gobj, struct media_entity, graph_obj)
+#define gobj_to_pad(gobj) \
+ container_of(gobj, struct media_pad, graph_obj)
+
+#define gobj_to_link(gobj) \
+ container_of(gobj, struct media_link, graph_obj)
+
void media_gobj_init(struct media_device *mdev,
enum media_gobj_type type,
struct media_gobj *gobj);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH RFC v5 8/8] [media] media: rename the function that create pad links
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
` (6 preceding siblings ...)
2015-08-18 20:04 ` [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal Mauro Carvalho Chehab
@ 2015-08-18 20:04 ` Mauro Carvalho Chehab
2015-08-19 8:38 ` Hans Verkuil
2015-08-18 20:08 ` [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
8 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:04 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Jonathan Corbet,
Kyungmin Park, Andrzej Hajda, Sakari Ailus, Sylwester Nawrocki,
Kukjin Kim, Krzysztof Kozlowski, Laurent Pinchart, Hyun Kwon,
Michal Simek, Sören Brinkmann, Greg Kroah-Hartman,
Matthias Schwarzott, Antti Palosaari, Hans Verkuil, Olli Salonen,
Tommi Rantala, Prabhakar Lad, Haneen Mohammed, Boris BREZILLON,
Tapasweni Pathak, Mahati Chamarthy, Jiayi Ye,
Navya Sri Nizamkari, Heena Sirwani, Wolfram Sang, linux-doc,
linux-arm-kernel, linux-samsung-soc, linux-sh, devel
Now that a link can be either between two different graph
objects, we'll need to add more functions to create links.
So, rename the existing one that create links only between
two pads as media_create_pad_link().
No functional changes.
This patch was created via this shell script:
for i in $(find drivers/media -name '*.[ch]' -type f) $(find drivers/staging/media -name '*.[ch]' -type f) $(find include/ -name '*.h' -type f) ; do sed s,media_entity_create_link,media_create_pad_link,g <$i >a && mv a $i; done
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 6903b2503577..b424de6c3bb3 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -199,7 +199,7 @@ pre-allocated and grows dynamically as needed.
Drivers create links by calling
- media_entity_create_link(struct media_entity *source, u16 source_pad,
+ media_create_pad_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad,
u32 flags);
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 2fdcbb5f000a..65f59f2124b4 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -412,16 +412,16 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
}
if (tuner && fe)
- media_entity_create_link(tuner, 0, fe, 0, 0);
+ media_create_pad_link(tuner, 0, fe, 0, 0);
if (fe && demux)
- media_entity_create_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
+ media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
if (demux && dvr)
- media_entity_create_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
+ media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
if (demux && ca)
- media_entity_create_link(demux, 1, ca, 0, MEDIA_LNK_FL_ENABLED);
+ media_create_pad_link(demux, 1, ca, 0, MEDIA_LNK_FL_ENABLED);
}
EXPORT_SYMBOL_GPL(dvb_create_media_graph);
#endif
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index 6d167428727d..c81bfbfea32f 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1482,11 +1482,11 @@ static int s5c73m3_oif_registered(struct v4l2_subdev *sd)
return ret;
}
- ret = media_entity_create_link(&state->sensor_sd.entity,
+ ret = media_create_pad_link(&state->sensor_sd.entity,
S5C73M3_ISP_PAD, &state->oif_sd.entity, OIF_ISP_PAD,
MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
- ret = media_entity_create_link(&state->sensor_sd.entity,
+ ret = media_create_pad_link(&state->sensor_sd.entity,
S5C73M3_JPEG_PAD, &state->oif_sd.entity, OIF_JPEG_PAD,
MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 30a9ca62e034..d3bff30bcb6f 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1756,7 +1756,7 @@ static int s5k5baf_registered(struct v4l2_subdev *sd)
v4l2_err(sd, "failed to register subdev %s\n",
state->cis_sd.name);
else
- ret = media_entity_create_link(&state->cis_sd.entity, PAD_CIS,
+ ret = media_create_pad_link(&state->cis_sd.entity, PAD_CIS,
&state->sd.entity, PAD_CIS,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 308613ea0aed..5aa49eb393a9 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2495,7 +2495,7 @@ static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
return rval;
}
- rval = media_entity_create_link(&this->sd.entity,
+ rval = media_create_pad_link(&this->sd.entity,
this->source_pad,
&last->sd.entity,
last->sink_pad,
@@ -2503,7 +2503,7 @@ static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
MEDIA_LNK_FL_IMMUTABLE);
if (rval) {
dev_err(&client->dev,
- "media_entity_create_link failed\n");
+ "media_create_pad_link failed\n");
return rval;
}
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 9e5ebc83ff76..b9304a634490 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -540,7 +540,7 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
}
int
-media_entity_create_link(struct media_entity *source, u16 source_pad,
+media_create_pad_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags)
{
struct media_link *link;
@@ -582,7 +582,7 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
return 0;
}
-EXPORT_SYMBOL_GPL(media_entity_create_link);
+EXPORT_SYMBOL_GPL(media_create_pad_link);
void __media_entity_remove_links(struct media_entity *entity)
{
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 4f5586a4cbff..3ba76940eef5 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -729,7 +729,7 @@ static int __fimc_md_create_fimc_sink_links(struct fimc_md *fmd,
flags = ((1 << i) & link_mask) ? MEDIA_LNK_FL_ENABLED : 0;
sink = &fmd->fimc[i]->vid_cap.subdev.entity;
- ret = media_entity_create_link(source, pad, sink,
+ ret = media_create_pad_link(source, pad, sink,
FIMC_SD_PAD_SINK_CAM, flags);
if (ret)
return ret;
@@ -749,7 +749,7 @@ static int __fimc_md_create_fimc_sink_links(struct fimc_md *fmd,
continue;
sink = &fmd->fimc_lite[i]->subdev.entity;
- ret = media_entity_create_link(source, pad, sink,
+ ret = media_create_pad_link(source, pad, sink,
FLITE_SD_PAD_SINK, 0);
if (ret)
return ret;
@@ -781,13 +781,13 @@ static int __fimc_md_create_flite_source_links(struct fimc_md *fmd)
source = &fimc->subdev.entity;
sink = &fimc->ve.vdev.entity;
/* FIMC-LITE's subdev and video node */
- ret = media_entity_create_link(source, FLITE_SD_PAD_SOURCE_DMA,
+ ret = media_create_pad_link(source, FLITE_SD_PAD_SOURCE_DMA,
sink, 0, 0);
if (ret)
break;
/* Link from FIMC-LITE to IS-ISP subdev */
sink = &fmd->fimc_is->isp.subdev.entity;
- ret = media_entity_create_link(source, FLITE_SD_PAD_SOURCE_ISP,
+ ret = media_create_pad_link(source, FLITE_SD_PAD_SOURCE_ISP,
sink, 0, 0);
if (ret)
break;
@@ -811,7 +811,7 @@ static int __fimc_md_create_fimc_is_links(struct fimc_md *fmd)
/* Link from FIMC-IS-ISP subdev to FIMC */
sink = &fmd->fimc[i]->vid_cap.subdev.entity;
- ret = media_entity_create_link(source, FIMC_ISP_SD_PAD_SRC_FIFO,
+ ret = media_create_pad_link(source, FIMC_ISP_SD_PAD_SRC_FIFO,
sink, FIMC_SD_PAD_SINK_FIFO, 0);
if (ret)
return ret;
@@ -824,7 +824,7 @@ static int __fimc_md_create_fimc_is_links(struct fimc_md *fmd)
if (sink->num_pads == 0)
return 0;
- return media_entity_create_link(source, FIMC_ISP_SD_PAD_SRC_DMA,
+ return media_create_pad_link(source, FIMC_ISP_SD_PAD_SRC_DMA,
sink, 0, 0);
}
@@ -873,7 +873,7 @@ static int fimc_md_create_links(struct fimc_md *fmd)
return -EINVAL;
pad = sensor->entity.num_pads - 1;
- ret = media_entity_create_link(&sensor->entity, pad,
+ ret = media_create_pad_link(&sensor->entity, pad,
&csis->entity, CSIS_PAD_SINK,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
@@ -927,7 +927,7 @@ static int fimc_md_create_links(struct fimc_md *fmd)
source = &fmd->fimc[i]->vid_cap.subdev.entity;
sink = &fmd->fimc[i]->vid_cap.ve.vdev.entity;
- ret = media_entity_create_link(source, FIMC_SD_PAD_SOURCE,
+ ret = media_create_pad_link(source, FIMC_SD_PAD_SOURCE,
sink, 0, flags);
if (ret)
break;
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 56e683b19a73..d6b2922ed55c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1862,7 +1862,7 @@ static int isp_link_entity(
return -EINVAL;
}
- return media_entity_create_link(entity, i, input, pad, flags);
+ return media_create_pad_link(entity, i, input, pad, flags);
}
static int isp_register_entities(struct isp_device *isp)
@@ -2001,51 +2001,51 @@ static int isp_initialize_modules(struct isp_device *isp)
}
/* Connect the submodules. */
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_csi2a.subdev.entity, CSI2_PAD_SOURCE,
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_ccp2.subdev.entity, CCP2_PAD_SOURCE,
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SOURCE_VP,
&isp->isp_prev.subdev.entity, PREV_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SOURCE_OF,
&isp->isp_res.subdev.entity, RESZ_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_prev.subdev.entity, PREV_PAD_SOURCE,
&isp->isp_res.subdev.entity, RESZ_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SOURCE_VP,
&isp->isp_aewb.subdev.entity, 0,
MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SOURCE_VP,
&isp->isp_af.subdev.entity, 0,
MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&isp->isp_ccdc.subdev.entity, CCDC_PAD_SOURCE_VP,
&isp->isp_hist.subdev.entity, 0,
MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 3b10304b580b..3fe425f254bb 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -2667,7 +2667,7 @@ static int ccdc_init_entities(struct isp_ccdc_device *ccdc)
goto error_video;
/* Connect the CCDC subdev to the video node. */
- ret = media_entity_create_link(&ccdc->subdev.entity, CCDC_PAD_SOURCE_OF,
+ ret = media_create_pad_link(&ccdc->subdev.entity, CCDC_PAD_SOURCE_OF,
&ccdc->video_out.video.entity, 0, 0);
if (ret < 0)
goto error_link;
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index e1b5f5bea541..b215eb5049d6 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -1100,7 +1100,7 @@ static int ccp2_init_entities(struct isp_ccp2_device *ccp2)
goto error_video;
/* Connect the video node to the ccp2 subdev. */
- ret = media_entity_create_link(&ccp2->video_in.video.entity, 0,
+ ret = media_create_pad_link(&ccp2->video_in.video.entity, 0,
&ccp2->subdev.entity, CCP2_PAD_SINK, 0);
if (ret < 0)
goto error_link;
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 6fff92f0813a..fcefc1e74881 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -1265,7 +1265,7 @@ static int csi2_init_entities(struct isp_csi2_device *csi2)
goto error_video;
/* Connect the CSI2 subdev to the video node. */
- ret = media_entity_create_link(&csi2->subdev.entity, CSI2_PAD_SOURCE,
+ ret = media_create_pad_link(&csi2->subdev.entity, CSI2_PAD_SOURCE,
&csi2->video_out.video.entity, 0, 0);
if (ret < 0)
goto error_link;
diff --git a/drivers/media/platform/omap3isp/isppreview.c b/drivers/media/platform/omap3isp/isppreview.c
index b440c6342ca4..ad38d20c7770 100644
--- a/drivers/media/platform/omap3isp/isppreview.c
+++ b/drivers/media/platform/omap3isp/isppreview.c
@@ -2312,12 +2312,12 @@ static int preview_init_entities(struct isp_prev_device *prev)
goto error_video_out;
/* Connect the video nodes to the previewer subdev. */
- ret = media_entity_create_link(&prev->video_in.video.entity, 0,
+ ret = media_create_pad_link(&prev->video_in.video.entity, 0,
&prev->subdev.entity, PREV_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(&prev->subdev.entity, PREV_PAD_SOURCE,
+ ret = media_create_pad_link(&prev->subdev.entity, PREV_PAD_SOURCE,
&prev->video_out.video.entity, 0, 0);
if (ret < 0)
goto error_link;
diff --git a/drivers/media/platform/omap3isp/ispresizer.c b/drivers/media/platform/omap3isp/ispresizer.c
index 3deb1ec4a973..b48ad4d4b834 100644
--- a/drivers/media/platform/omap3isp/ispresizer.c
+++ b/drivers/media/platform/omap3isp/ispresizer.c
@@ -1756,12 +1756,12 @@ static int resizer_init_entities(struct isp_res_device *res)
res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;
/* Connect the video nodes to the resizer subdev. */
- ret = media_entity_create_link(&res->video_in.video.entity, 0,
+ ret = media_create_pad_link(&res->video_in.video.entity, 0,
&res->subdev.entity, RESZ_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(&res->subdev.entity, RESZ_PAD_SOURCE,
+ ret = media_create_pad_link(&res->subdev.entity, RESZ_PAD_SOURCE,
&res->video_out.video.entity, 0, 0);
if (ret < 0)
goto error_link;
diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
index f47b332f0418..3e33c60be004 100644
--- a/drivers/media/platform/s3c-camif/camif-core.c
+++ b/drivers/media/platform/s3c-camif/camif-core.c
@@ -263,7 +263,7 @@ static int camif_create_media_links(struct camif_dev *camif)
{
int i, ret;
- ret = media_entity_create_link(&camif->sensor.sd->entity, 0,
+ ret = media_create_pad_link(&camif->sensor.sd->entity, 0,
&camif->subdev.entity, CAMIF_SD_PAD_SINK,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
@@ -271,7 +271,7 @@ static int camif_create_media_links(struct camif_dev *camif)
return ret;
for (i = 1; i < CAMIF_SD_PADS_NUM && !ret; i++) {
- ret = media_entity_create_link(&camif->subdev.entity, i,
+ ret = media_create_pad_link(&camif->subdev.entity, i,
&camif->vp[i - 1].vdev.entity, 0,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 4e61886384e3..9cd94a76a9ed 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -101,7 +101,7 @@ static int vsp1_create_links(struct vsp1_device *vsp1, struct vsp1_entity *sink)
if (!(entity->pads[pad].flags & MEDIA_PAD_FL_SINK))
continue;
- ret = media_entity_create_link(&source->subdev.entity,
+ ret = media_create_pad_link(&source->subdev.entity,
source->source_pad,
entity, pad, flags);
if (ret < 0)
@@ -262,7 +262,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
}
if (vsp1->pdata.features & VSP1_HAS_LIF) {
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&vsp1->wpf[0]->entity.subdev.entity, RWPF_PAD_SOURCE,
&vsp1->lif->entity.subdev.entity, LIF_PAD_SINK, 0);
if (ret < 0)
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 3294529a3108..b60a528a8fe8 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -278,7 +278,7 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device *vsp1, unsigned int index)
rpf->entity.video = video;
/* Connect the video device to the RPF. */
- ret = media_entity_create_link(&rpf->video.video.entity, 0,
+ ret = media_create_pad_link(&rpf->video.video.entity, 0,
&rpf->entity.subdev.entity,
RWPF_PAD_SINK,
MEDIA_LNK_FL_ENABLED |
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index 1d2b3a2f1573..d39aa4b8aea1 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -284,7 +284,7 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
if (!(vsp1->pdata.features & VSP1_HAS_LIF) || index != 0)
flags |= MEDIA_LNK_FL_IMMUTABLE;
- ret = media_entity_create_link(&wpf->entity.subdev.entity,
+ ret = media_create_pad_link(&wpf->entity.subdev.entity,
RWPF_PAD_SOURCE,
&wpf->video.video.entity, 0, flags);
if (ret < 0)
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index 7b7cb9c28d2c..79d4be7ce9a5 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -156,7 +156,7 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
local->name, local_pad->index,
remote->name, remote_pad->index);
- ret = media_entity_create_link(local, local_pad->index,
+ ret = media_create_pad_link(local, local_pad->index,
remote, remote_pad->index,
link_flags);
if (ret < 0) {
@@ -270,7 +270,7 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
source->name, source_pad->index,
sink->name, sink_pad->index);
- ret = media_entity_create_link(source, source_pad->index,
+ ret = media_create_pad_link(source, source_pad->index,
sink, sink_pad->index,
link_flags);
if (ret < 0) {
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 4a117a58c39a..3b5c9ae39ad3 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1264,11 +1264,11 @@ static void cx231xx_create_media_graph(struct cx231xx *dev)
return;
if (tuner)
- media_entity_create_link(tuner, 0, decoder, 0,
+ media_create_pad_link(tuner, 0, decoder, 0,
MEDIA_LNK_FL_ENABLED);
- media_entity_create_link(decoder, 1, &dev->vdev.entity, 0,
+ media_create_pad_link(decoder, 1, &dev->vdev.entity, 0,
MEDIA_LNK_FL_ENABLED);
- media_entity_create_link(decoder, 2, &dev->vbi_dev.entity, 0,
+ media_create_pad_link(decoder, 2, &dev->vbi_dev.entity, 0,
MEDIA_LNK_FL_ENABLED);
#endif
}
diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index 245445491516..429e428ccd93 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -56,7 +56,7 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain,
continue;
remote_pad = remote->num_pads - 1;
- ret = media_entity_create_link(source, remote_pad,
+ ret = media_create_pad_link(source, remote_pad,
sink, i, flags);
if (ret < 0)
return ret;
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
index 8fb676186898..d96bdaaae50e 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
@@ -971,7 +971,7 @@ vpfe_ipipeif_register_entities(struct vpfe_ipipeif_device *ipipeif,
ipipeif->video_in.vpfe_dev = vpfe_dev;
flags = 0;
- ret = media_entity_create_link(&ipipeif->video_in.video_dev.entity, 0,
+ ret = media_create_pad_link(&ipipeif->video_in.video_dev.entity, 0,
&ipipeif->subdev.entity, 0, flags);
if (ret < 0)
goto fail;
diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c b/drivers/staging/media/davinci_vpfe/dm365_isif.c
index b1f01adfa7c8..df77288b0ec0 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
@@ -1817,7 +1817,7 @@ int vpfe_isif_register_entities(struct vpfe_isif_device *isif,
isif->video_out.vpfe_dev = vpfe_dev;
flags = 0;
/* connect isif to video node */
- ret = media_entity_create_link(&isif->subdev.entity, 1,
+ ret = media_create_pad_link(&isif->subdev.entity, 1,
&isif->video_out.video_dev.entity,
0, flags);
if (ret < 0)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
index 692789aa22f4..ae942de3a23d 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
@@ -1831,27 +1831,27 @@ int vpfe_resizer_register_entities(struct vpfe_resizer_device *resizer,
resizer->resizer_b.video_out.vpfe_dev = vpfe_dev;
/* create link between Resizer Crop----> Resizer A*/
- ret = media_entity_create_link(&resizer->crop_resizer.subdev.entity, 1,
+ ret = media_create_pad_link(&resizer->crop_resizer.subdev.entity, 1,
&resizer->resizer_a.subdev.entity,
0, flags);
if (ret < 0)
goto out_create_link;
/* create link between Resizer Crop----> Resizer B*/
- ret = media_entity_create_link(&resizer->crop_resizer.subdev.entity, 2,
+ ret = media_create_pad_link(&resizer->crop_resizer.subdev.entity, 2,
&resizer->resizer_b.subdev.entity,
0, flags);
if (ret < 0)
goto out_create_link;
/* create link between Resizer A ----> video out */
- ret = media_entity_create_link(&resizer->resizer_a.subdev.entity, 1,
+ ret = media_create_pad_link(&resizer->resizer_a.subdev.entity, 1,
&resizer->resizer_a.video_out.video_dev.entity, 0, flags);
if (ret < 0)
goto out_create_link;
/* create link between Resizer B ----> video out */
- ret = media_entity_create_link(&resizer->resizer_b.subdev.entity, 1,
+ ret = media_create_pad_link(&resizer->resizer_b.subdev.entity, 1,
&resizer->resizer_b.video_out.video_dev.entity, 0, flags);
if (ret < 0)
goto out_create_link;
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
index 57426199ad7a..08c8a5f967d3 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
@@ -445,32 +445,32 @@ static int vpfe_register_entities(struct vpfe_device *vpfe_dev)
/* if entity has no pads (ex: amplifier),
cant establish link */
if (vpfe_dev->sd[i]->entity.num_pads) {
- ret = media_entity_create_link(&vpfe_dev->sd[i]->entity,
+ ret = media_create_pad_link(&vpfe_dev->sd[i]->entity,
0, &vpfe_dev->vpfe_isif.subdev.entity,
0, flags);
if (ret < 0)
goto out_resizer_register;
}
- ret = media_entity_create_link(&vpfe_dev->vpfe_isif.subdev.entity, 1,
+ ret = media_create_pad_link(&vpfe_dev->vpfe_isif.subdev.entity, 1,
&vpfe_dev->vpfe_ipipeif.subdev.entity,
0, flags);
if (ret < 0)
goto out_resizer_register;
- ret = media_entity_create_link(&vpfe_dev->vpfe_ipipeif.subdev.entity, 1,
+ ret = media_create_pad_link(&vpfe_dev->vpfe_ipipeif.subdev.entity, 1,
&vpfe_dev->vpfe_ipipe.subdev.entity,
0, flags);
if (ret < 0)
goto out_resizer_register;
- ret = media_entity_create_link(&vpfe_dev->vpfe_ipipe.subdev.entity,
+ ret = media_create_pad_link(&vpfe_dev->vpfe_ipipe.subdev.entity,
1, &vpfe_dev->vpfe_resizer.crop_resizer.subdev.entity,
0, flags);
if (ret < 0)
goto out_resizer_register;
- ret = media_entity_create_link(&vpfe_dev->vpfe_ipipeif.subdev.entity, 1,
+ ret = media_create_pad_link(&vpfe_dev->vpfe_ipipeif.subdev.entity, 1,
&vpfe_dev->vpfe_resizer.crop_resizer.subdev.entity,
0, flags);
if (ret < 0)
diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index 9bfb725b9986..f32ab7b98ae2 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -1259,7 +1259,7 @@ static int iss_register_entities(struct iss_device *iss)
goto done;
}
- ret = media_entity_create_link(&sensor->entity, 0, input, pad,
+ ret = media_create_pad_link(&sensor->entity, 0, input, pad,
flags);
if (ret < 0)
goto done;
@@ -1317,31 +1317,31 @@ static int iss_initialize_modules(struct iss_device *iss)
}
/* Connect the submodules. */
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&iss->csi2a.subdev.entity, CSI2_PAD_SOURCE,
&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&iss->csi2b.subdev.entity, CSI2_PAD_SOURCE,
&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&iss->ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
&iss->ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
if (ret < 0)
goto error_link;
- ret = media_entity_create_link(
+ ret = media_create_pad_link(
&iss->ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
&iss->resizer.subdev.entity, RESIZER_PAD_SINK, 0);
if (ret < 0)
diff --git a/drivers/staging/media/omap4iss/iss_csi2.c b/drivers/staging/media/omap4iss/iss_csi2.c
index e936cfc218cb..6b4dcbfa9425 100644
--- a/drivers/staging/media/omap4iss/iss_csi2.c
+++ b/drivers/staging/media/omap4iss/iss_csi2.c
@@ -1291,7 +1291,7 @@ static int csi2_init_entities(struct iss_csi2_device *csi2, const char *subname)
goto error_video;
/* Connect the CSI2 subdev to the video node. */
- ret = media_entity_create_link(&csi2->subdev.entity, CSI2_PAD_SOURCE,
+ ret = media_create_pad_link(&csi2->subdev.entity, CSI2_PAD_SOURCE,
&csi2->video_out.video.entity, 0, 0);
if (ret < 0)
goto error_link;
diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.c b/drivers/staging/media/omap4iss/iss_ipipeif.c
index be5f80d7b5dc..44c432ef2ac5 100644
--- a/drivers/staging/media/omap4iss/iss_ipipeif.c
+++ b/drivers/staging/media/omap4iss/iss_ipipeif.c
@@ -762,7 +762,7 @@ static int ipipeif_init_entities(struct iss_ipipeif_device *ipipeif)
return ret;
/* Connect the IPIPEIF subdev to the video node. */
- ret = media_entity_create_link(&ipipeif->subdev.entity,
+ ret = media_create_pad_link(&ipipeif->subdev.entity,
IPIPEIF_PAD_SOURCE_ISIF_SF,
&ipipeif->video_out.video.entity, 0, 0);
if (ret < 0)
diff --git a/drivers/staging/media/omap4iss/iss_resizer.c b/drivers/staging/media/omap4iss/iss_resizer.c
index 91e724085dba..b659e465cb56 100644
--- a/drivers/staging/media/omap4iss/iss_resizer.c
+++ b/drivers/staging/media/omap4iss/iss_resizer.c
@@ -806,7 +806,7 @@ static int resizer_init_entities(struct iss_resizer_device *resizer)
return ret;
/* Connect the RESIZER subdev to the video node. */
- ret = media_entity_create_link(&resizer->subdev.entity,
+ ret = media_create_pad_link(&resizer->subdev.entity,
RESIZER_PAD_SOURCE_MEM,
&resizer->video_out.video.entity, 0, 0);
if (ret < 0)
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index db1f54f09723..534e600ffe50 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -206,7 +206,7 @@ int media_entity_init(struct media_entity *entity, u16 num_pads,
struct media_pad *pads);
void media_entity_cleanup(struct media_entity *entity);
-int media_entity_create_link(struct media_entity *source, u16 source_pad,
+int media_create_pad_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags);
void __media_entity_remove_links(struct media_entity *entity);
void media_entity_remove_links(struct media_entity *entity);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 0/8] MC preparation patches
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
` (7 preceding siblings ...)
2015-08-18 20:04 ` [PATCH RFC v5 8/8] [media] media: rename the function that create pad links Mauro Carvalho Chehab
@ 2015-08-18 20:08 ` Mauro Carvalho Chehab
8 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-18 20:08 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
Em Tue, 18 Aug 2015 17:04:13 -0300
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:
Hmm... I forgot to remove the "RFC" prefix. For me, this patch series is
OK to be merged at the media-controller topic branch.
> Those are the initial patches from my RFCv3 series of MC changes,
> plus one additional renaming patch and two debug ones. They address
> the comments received on PATCH v4 series.
>
> The first 5 patches on this series ensures that all existing object
> types (entities, pads and links) will have an unique ID, as agreed
> at the MC workshop.
>
> The next two patches add two debug functions, that helps with the
> tests of the MC changes. Both are enabled only if DEBUG or dynamic
> debug is enabled.
>
> The first one just help to identify when the media_device register,
> remove and unregister functions are called. It helps to identify if
> those events happen before or after object creation.
>
> The second one is more interesting: it hooks at the object init and
> remove functions and dump what's there at the new object when it
> got created. This is very useful to test the future patches, as we'll
> be able to track any topology changes.
>
> Also, it demonstates the capability of the functions
> media_gobj_init() and media_gobj_remove() to track topology
> changes. Tracking topology changes is fundamental for the new API,
> in order to implement G_TOPOLOGY ioctl. They should contain, in
> the future, a callback to warn the several drivers envolved at the
> MC topology build about topology changes.
>
> The last patch on this preparation series is just a renaming patch,
> that will avoid mess when future patches introduce the
> entity->interface links.
>
> They're tested using the cx231xx V4L2 support.
>
> Those are the new debug messages when the device is probed:
>
> [ 2684.265619] cx231xx 3-2.4:1.1: Media device registered
> [ 2684.357670] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000001 entity#1 'cx25840 19-0044'
> [ 2684.359191] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000001 cx25840 19-0044 pad#1
> [ 2684.360693] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000002 cx25840 19-0044 pad#2
> [ 2684.362208] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000003 cx25840 19-0044 pad#3
>
> [ 2686.437870] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000002 entity#2 '(tuner unset)'
> [ 2686.439474] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000004 (tuner unset) pad#4
>
> [ 2688.450710] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000003 entity#3 'cx231xx #0 video'
> [ 2688.452395] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000005 cx231xx #0 video pad#5
>
> [ 2688.455892] cx231xx 3-2.4:1.1: media_gobj_init: id 0x00000004 entity#4 'cx231xx #0 vbi'
> [ 2688.457632] cx231xx 3-2.4:1.1: media_gobj_init: id 0x01000006 cx231xx #0 vbi pad#6
>
> [ 2688.500233] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000001 link#1: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
> [ 2688.501830] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000002 link#2: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
> [ 2688.503415] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000003 link#3: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
> [ 2688.504977] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000004 link#4: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
> [ 2688.506505] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000005 link#5: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
> [ 2688.508074] cx231xx 3-2.4:1.1: media_gobj_init: id 0x02000006 link#6: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
>
> Those are the new debug messages when the device is removed:
>
> [ 2953.872780] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000002 link#2: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
> [ 2953.874470] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000003 link#3: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
> [ 2953.876199] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000005 link#5: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
> [ 2953.877830] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000001 cx25840 19-0044 pad#1
> [ 2953.879454] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000002 cx25840 19-0044 pad#2
> [ 2953.881020] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000003 cx25840 19-0044 pad#3
> [ 2953.882569] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000001 entity#1 'cx25840 19-0044'
>
> [ 2953.884093] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000001 link#1: 'NXP TDA18271HD' pad#4 ==> 'cx25840 19-0044' pad#1
> [ 2953.885611] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000004 NXP TDA18271HD pad#4
> [ 2953.887200] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000002 entity#2 'NXP TDA18271HD'
>
> [ 2953.888638] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000004 link#4: 'cx25840 19-0044' pad#2 ==> 'cx231xx #0 video' pad#5
> [ 2953.890093] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000005 cx231xx #0 video pad#5
> [ 2953.891549] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000003 entity#3 'cx231xx #0 video'
>
> [ 2953.893006] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x02000006 link#6: 'cx25840 19-0044' pad#3 ==> 'cx231xx #0 vbi' pad#6
> [ 2953.894471] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x01000006 cx231xx #0 vbi pad#6
> [ 2953.895955] cx231xx 3-2.4:1.1: media_gobj_remove: id 0x00000004 entity#4 'cx231xx #0 vbi'
> [ 2953.897505] cx231xx 3-2.4:1.1: Media device released
> [ 2953.898924] cx231xx 3-2.4:1.1: Media device unregistered
>
> It should be noticed that both links and backlinks are reported
> on the above. That's because now they're implemented as two separate
> links right now.
>
> Mauro Carvalho Chehab (8):
> [media] media: create a macro to get entity ID
> [media] media: add a common struct to be embed on media graph objects
> [media] media: use media_gobj inside entities
> [media] media: use media_gobj inside pads
> [media] media: use media_gobj inside links
> [media] media: add messages when media device gets (un)registered
> [media] media: add a debug message to warn about gobj creation/removal
> [media] media: rename the function that create pad links
>
> Documentation/media-framework.txt | 2 +-
> drivers/media/dvb-core/dvbdev.c | 8 +-
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 4 +-
> drivers/media/i2c/s5k5baf.c | 2 +-
> drivers/media/i2c/smiapp/smiapp-core.c | 4 +-
> drivers/media/media-device.c | 41 +++++--
> drivers/media/media-entity.c | 123 ++++++++++++++++++++-
> drivers/media/platform/exynos4-is/media-dev.c | 16 +--
> drivers/media/platform/omap3isp/isp.c | 18 +--
> drivers/media/platform/omap3isp/ispccdc.c | 2 +-
> drivers/media/platform/omap3isp/ispccp2.c | 2 +-
> drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
> drivers/media/platform/omap3isp/isppreview.c | 4 +-
> drivers/media/platform/omap3isp/ispresizer.c | 4 +-
> drivers/media/platform/s3c-camif/camif-core.c | 4 +-
> drivers/media/platform/vsp1/vsp1_drv.c | 4 +-
> drivers/media/platform/vsp1/vsp1_rpf.c | 2 +-
> drivers/media/platform/vsp1/vsp1_video.c | 4 +-
> drivers/media/platform/vsp1/vsp1_wpf.c | 2 +-
> drivers/media/platform/xilinx/xilinx-vipp.c | 4 +-
> drivers/media/usb/cx231xx/cx231xx-cards.c | 6 +-
> drivers/media/usb/uvc/uvc_entity.c | 2 +-
> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 2 +-
> drivers/staging/media/davinci_vpfe/dm365_isif.c | 2 +-
> drivers/staging/media/davinci_vpfe/dm365_resizer.c | 8 +-
> .../staging/media/davinci_vpfe/vpfe_mc_capture.c | 10 +-
> drivers/staging/media/omap4iss/iss.c | 12 +-
> drivers/staging/media/omap4iss/iss_csi2.c | 2 +-
> drivers/staging/media/omap4iss/iss_ipipeif.c | 2 +-
> drivers/staging/media/omap4iss/iss_resizer.c | 2 +-
> include/media/media-device.h | 7 +-
> include/media/media-entity.h | 82 +++++++++++++-
> 32 files changed, 302 insertions(+), 87 deletions(-)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects
2015-08-18 20:04 ` [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects Mauro Carvalho Chehab
@ 2015-08-19 8:17 ` Hans Verkuil
2015-08-19 9:19 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2015-08-19 8:17 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
> Due to the MC API proposed changes, we'll need to have an unique
> object ID for all graph objects, and have some shared fields
> that will be common on all media graph objects.
>
> Right now, the only common object is the object ID, but other
> fields will be added later on.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index cb0ac4e0dfa5..4834172bf6f8 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -27,6 +27,38 @@
> #include <media/media-device.h>
>
> /**
> + * media_gobj_init - Initialize a graph object
> + *
> + * @mdev: Pointer to the media_device that contains the object
> + * @type: Type of the object
> + * @gobj: Pointer to the object
> + *
> + * This routine initializes the embedded struct media_gobj inside a
> + * media graph object. It is called automatically if media_*_create()
> + * calls are used. However, if the object (entity, link, pad, interface)
> + * is embedded on some other object, this function should be called before
> + * registering the object at the media controller.
> + */
> +void media_gobj_init(struct media_device *mdev,
> + enum media_gobj_type type,
> + struct media_gobj *gobj)
> +{
> + /* For now, nothing to do */
> +}
> +
> +/**
> + * media_gobj_remove - Stop using a graph object on a media device
> + *
> + * @graph_obj: Pointer to the object
> + *
> + * This should be called at media_device_unregister_*() routines
> + */
> +void media_gobj_remove(struct media_gobj *gobj)
> +{
> + /* For now, nothing to do */
> +}
> +
> +/**
> * media_entity_init - Initialize a media entity
> *
> * @num_pads: Total number of sink and source pads.
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 0a66fc225559..762593c7424f 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -28,6 +28,37 @@
> #include <linux/list.h>
> #include <linux/media.h>
>
> +/* Enums used internally at the media controller to represent graphs */
> +
> +/**
> + * enum media_gobj_type - type of a graph element
> + *
> + */
> +enum media_gobj_type {
> + /* FIXME: add the types here, as we embed media_gobj */
> + MEDIA_GRAPH_NONE
> +};
> +
> +#define BITS_PER_TYPE 8
> +#define BITS_PER_LOCAL_ID (32 - BITS_PER_TYPE)
I think that these two defines should be prefixed with MEDIA_.
They are a bit too generic for a header that's going to be used in lots
of drivers.
I also think that the local ID mask should be a define as well:
#define MEDIA_LOCAL_ID_MASK GENMASK(BITS_PER_LOCAL_ID - 1, 0)
That will make the code where this mask is used a bit more readable.
> +
> +/* Structs to represent the objects that belong to a media graph */
> +
> +/**
> + * struct media_gobj - Define a graph object.
> + *
> + * @id: Non-zero object ID identifier. The ID should be unique
> + * inside a media_device, as it is composed by
> + * BITS_PER_TYPE to store the type plus BITS_PER_LOCAL_ID
> + * to store a per-type ID (called as "local ID").
> + *
> + * All elements on the media graph should have this struct embedded
> + */
> +struct media_gobj {
> + u32 id;
> +};
> +
> +
> struct media_pipeline {
> };
>
> @@ -118,6 +149,26 @@ static inline u32 media_entity_id(struct media_entity *entity)
> return entity->id;
> }
>
> +static inline enum media_gobj_type media_type(struct media_gobj *gobj)
> +{
> + return gobj->id >> BITS_PER_LOCAL_ID;
> +}
> +
> +static inline u32 media_localid(struct media_gobj *gobj)
> +{
> + return gobj->id & GENMASK(BITS_PER_LOCAL_ID - 1, 0);
> +}
> +
> +static inline u32 media_gobj_gen_id(enum media_gobj_type type, u32 local_id)
> +{
> + u32 id;
> +
> + id = type << BITS_PER_LOCAL_ID;
> + id |= GENMASK(BITS_PER_LOCAL_ID - 1, 0) & local_id;
I'd swap this: id |= local_id & MEDIA_LOCAL_ID_MASK;
It feels more natural that way and it is consistent with media_localid() above.
Regards,
Hans
> +
> + return id;
> +}
> +
> #define MEDIA_ENTITY_ENUM_MAX_DEPTH 16
> #define MEDIA_ENTITY_ENUM_MAX_ID 64
>
> @@ -131,6 +182,14 @@ struct media_entity_graph {
> int top;
> };
>
> +#define gobj_to_entity(gobj) \
> + container_of(gobj, struct media_entity, graph_obj)
> +
> +void media_gobj_init(struct media_device *mdev,
> + enum media_gobj_type type,
> + struct media_gobj *gobj);
> +void media_gobj_remove(struct media_gobj *gobj);
> +
> int media_entity_init(struct media_entity *entity, u16 num_pads,
> struct media_pad *pads);
> void media_entity_cleanup(struct media_entity *entity);
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 5/8] [media] media: use media_gobj inside links
2015-08-18 20:04 ` [PATCH RFC v5 5/8] [media] media: use media_gobj inside links Mauro Carvalho Chehab
@ 2015-08-19 8:18 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2015-08-19 8:18 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
> Just like entities and pads, links also need to have unique
> Object IDs along a given media controller.
>
> So, let's add a media_gobj inside it and initialize
> the object then a new link is created.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks!
Hans
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 24551d4ddfb8..7f512223249b 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -436,6 +436,13 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
> list_add_tail(&entity->list, &mdev->entities);
>
> + /*
> + * Initialize objects at the links
> + * in the case where links got created before entity register
> + */
> + for (i = 0; i < entity->num_links; i++)
> + media_gobj_init(mdev, MEDIA_GRAPH_LINK,
> + &entity->links[i].graph_obj);
> /* Initialize objects at the pads */
> for (i = 0; i < entity->num_pads; i++)
> media_gobj_init(mdev, MEDIA_GRAPH_PAD,
> @@ -463,6 +470,8 @@ void media_device_unregister_entity(struct media_entity *entity)
> return;
>
> spin_lock(&mdev->lock);
> + for (i = 0; i < entity->num_links; i++)
> + media_gobj_remove(&entity->links[i].graph_obj);
> for (i = 0; i < entity->num_pads; i++)
> media_gobj_remove(&entity->pads[i].graph_obj);
> media_gobj_remove(&entity->graph_obj);
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 377c6655c5d0..a6be50e04736 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -51,6 +51,9 @@ void media_gobj_init(struct media_device *mdev,
> case MEDIA_GRAPH_PAD:
> gobj->id = media_gobj_gen_id(type, ++mdev->pad_id);
> break;
> + case MEDIA_GRAPH_LINK:
> + gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
> + break;
> }
> }
>
> @@ -469,6 +472,13 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
> entity->links = links;
> }
>
> + /* Initialize graph object embedded at the new link */
> + if (entity->parent)
> + media_gobj_init(entity->parent, MEDIA_GRAPH_LINK,
> + &entity->links[entity->num_links].graph_obj);
> + else
> + pr_warn("Link created before entity register!\n");
> +
> return &entity->links[entity->num_links++];
> }
>
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 4b5d1ee2b67e..e0f63c0da2dd 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -43,6 +43,7 @@ struct device;
> * @driver_version: Device driver version
> * @entity_id: Unique ID used on the last entity registered
> * @pad_id: Unique ID used on the last pad registered
> + * @link_id: Unique ID used on the last link registered
> * @entities: List of registered entities
> * @lock: Entities list lock
> * @graph_mutex: Entities graph operation lock
> @@ -71,6 +72,7 @@ struct media_device {
>
> u32 entity_id;
> u32 pad_id;
> + u32 link_id;
>
> struct list_head entities;
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 01ae1e320e36..2ffe015629fa 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -35,10 +35,12 @@
> *
> * @MEDIA_GRAPH_ENTITY: Identify a media entity
> * @MEDIA_GRAPH_PAD: Identify a media pad
> + * @MEDIA_GRAPH_LINK: Identify a media link
> */
> enum media_gobj_type {
> MEDIA_GRAPH_ENTITY,
> MEDIA_GRAPH_PAD,
> + MEDIA_GRAPH_LINK,
> };
>
> #define BITS_PER_TYPE 8
> @@ -65,6 +67,7 @@ struct media_pipeline {
> };
>
> struct media_link {
> + struct media_gobj graph_obj;
> struct media_pad *source; /* Source pad */
> struct media_pad *sink; /* Sink pad */
> struct media_link *reverse; /* Link in the reverse direction */
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal
2015-08-18 20:04 ` [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal Mauro Carvalho Chehab
@ 2015-08-19 8:36 ` Hans Verkuil
2015-08-19 9:50 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2015-08-19 8:36 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
> It helps to check if the media controller is doing the
> right thing with the object creation and removal.
>
> No extra code/data will be produced if DEBUG or
> CONFIG_DYNAMIC_DEBUG is not enabled.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index a6be50e04736..9e5ebc83ff76 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -27,6 +27,67 @@
> #include <media/media-device.h>
>
> /**
> + * dev_dbg_obj - Prints in debug mode a change on some object
> + *
> + * @event_name: Name of the event to report. Could be __func__
> + * @gobj: Pointer to the object
> + *
> + * Enabled only if DEBUG or CONFIG_DYNAMIC_DEBUG. Otherwise, it
> + * won't produce any code.
> + */
> +static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
> +{
> +#if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
> + static const char *gobj_type[] = {
> + [MEDIA_GRAPH_ENTITY] = "entity",
> + [MEDIA_GRAPH_PAD] = "pad",
> + [MEDIA_GRAPH_LINK] = "link",
> + };
> +
> + switch (media_type(gobj)) {
> + case MEDIA_GRAPH_ENTITY:
> + dev_dbg(gobj->mdev->dev,
> + "%s: id 0x%08x %s#%d '%s'\n",
> + event_name, gobj->id,
> + gobj_type[media_type(gobj)],
You can just fill in "entity" in the string since this is always an entity in
this case.
> + media_localid(gobj),
> + gobj_to_entity(gobj)->name);
> + break;
> + case MEDIA_GRAPH_LINK:
> + {
> + struct media_link *link = gobj_to_link(gobj);
> +
> + dev_dbg(gobj->mdev->dev,
> + "%s: id 0x%08x %s#%d: '%s' %s#%d ==> '%s' %s#%d\n",
> + event_name, gobj->id,
> + gobj_type[media_type(gobj)],
Ditto.
> + media_localid(gobj),
> +
> + link->source->entity->name,
> + gobj_type[media_type(&link->source->graph_obj)],
> + media_localid(&link->source->graph_obj),
> +
> + link->sink->entity->name,
> + gobj_type[media_type(&link->sink->graph_obj)],
> + media_localid(&link->sink->graph_obj));
> + break;
> + }
> + case MEDIA_GRAPH_PAD:
> + {
> + struct media_pad *pad = gobj_to_pad(gobj);
> +
> + dev_dbg(gobj->mdev->dev,
> + "%s: id 0x%08x %s %s#%d\n",
> + event_name, gobj->id,
> + pad->entity->name,
> + gobj_type[media_type(gobj)],
Ditto.
> + media_localid(gobj));
> + }
I would add a default case here with a WARN(1) or something like that to make
it very clear that someone forgot to extend this switch after adding a new
object type.
In the LINK case there is no check for unknown object types in the gobj_type[]
lookups. I think it is overkill to add a check for that since the objects that
are linked are going to be created first, so they would trigger the WARN(1) in
the default case first if they are using unhandled types. That should give
enough information for the developer.
> + }
> +#endif
> +}
> +
> +/**
> * media_gobj_init - Initialize a graph object
> *
> * @mdev: Pointer to the media_device that contains the object
> @@ -43,6 +104,8 @@ void media_gobj_init(struct media_device *mdev,
> enum media_gobj_type type,
> struct media_gobj *gobj)
> {
> + gobj->mdev = mdev;
> +
> /* Create a per-type unique object ID */
> switch (type) {
> case MEDIA_GRAPH_ENTITY:
> @@ -55,6 +118,7 @@ void media_gobj_init(struct media_device *mdev,
> gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
> break;
> }
> + dev_dbg_obj(__func__, gobj);
> }
>
> /**
> @@ -66,7 +130,7 @@ void media_gobj_init(struct media_device *mdev,
> */
> void media_gobj_remove(struct media_gobj *gobj)
> {
> - /* For now, nothing to do */
> + dev_dbg_obj(__func__, gobj);
> }
>
> /**
> @@ -472,13 +536,6 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
> entity->links = links;
> }
>
> - /* Initialize graph object embedded at the new link */
> - if (entity->parent)
> - media_gobj_init(entity->parent, MEDIA_GRAPH_LINK,
> - &entity->links[entity->num_links].graph_obj);
> - else
> - pr_warn("Link created before entity register!\n");
> -
This is confusing: this was added in patch 5 and now it is removed again. This doesn't
seem to have anything to do with adding this debugging facility.
> return &entity->links[entity->num_links++];
> }
>
> @@ -501,6 +558,8 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
> link->sink = &sink->pads[sink_pad];
> link->flags = flags;
>
> + media_gobj_init(source->parent, MEDIA_GRAPH_LINK, &link->graph_obj);
> +
Same for this,
> /* Create the backlink. Backlinks are used to help graph traversal and
> * are not reported to userspace.
> */
> @@ -514,6 +573,8 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
> backlink->sink = &sink->pads[sink_pad];
> backlink->flags = flags;
>
> + media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
> +
and this.
Shouldn't these three changes be moved to patch 5?
> link->reverse = backlink;
> backlink->reverse = link;
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 2ffe015629fa..db1f54f09723 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -59,6 +59,7 @@ enum media_gobj_type {
> * All elements on the media graph should have this struct embedded
> */
> struct media_gobj {
> + struct media_device *mdev;
If you add this here, then the parent pointer in struct media_entity should be
removed to avoid having duplicate fields.
I understand why you add this and I don't object, but I would split this change
off into a separate patch that also includes removing the media_entity parent
field.
Regards,
Hans
> u32 id;
> };
>
> @@ -190,6 +191,12 @@ struct media_entity_graph {
> #define gobj_to_entity(gobj) \
> container_of(gobj, struct media_entity, graph_obj)
>
> +#define gobj_to_pad(gobj) \
> + container_of(gobj, struct media_pad, graph_obj)
> +
> +#define gobj_to_link(gobj) \
> + container_of(gobj, struct media_link, graph_obj)
> +
> void media_gobj_init(struct media_device *mdev,
> enum media_gobj_type type,
> struct media_gobj *gobj);
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 8/8] [media] media: rename the function that create pad links
2015-08-18 20:04 ` [PATCH RFC v5 8/8] [media] media: rename the function that create pad links Mauro Carvalho Chehab
@ 2015-08-19 8:38 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2015-08-19 8:38 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Jonathan Corbet, Kyungmin Park,
Andrzej Hajda, Sakari Ailus, Sylwester Nawrocki, Kukjin Kim,
Krzysztof Kozlowski, Laurent Pinchart, Hyun Kwon, Michal Simek,
Sören Brinkmann, Greg Kroah-Hartman, Matthias Schwarzott,
Antti Palosaari, Olli Salonen, Tommi Rantala, Prabhakar Lad,
Haneen Mohammed, Boris BREZILLON, Tapasweni Pathak,
Mahati Chamarthy, Jiayi Ye, Navya Sri Nizamkari, Heena Sirwani,
Wolfram Sang, linux-doc, linux-arm-kernel, linux-samsung-soc,
linux-sh, devel
On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
> Now that a link can be either between two different graph
> objects, we'll need to add more functions to create links.
> So, rename the existing one that create links only between
> two pads as media_create_pad_link().
>
> No functional changes.
>
> This patch was created via this shell script:
> for i in $(find drivers/media -name '*.[ch]' -type f) $(find drivers/staging/media -name '*.[ch]' -type f) $(find include/ -name '*.h' -type f) ; do sed s,media_entity_create_link,media_create_pad_link,g <$i >a && mv a $i; done
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks!
Hans
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects
2015-08-19 8:17 ` Hans Verkuil
@ 2015-08-19 9:19 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-19 9:19 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Em Wed, 19 Aug 2015 10:17:46 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
> > Due to the MC API proposed changes, we'll need to have an unique
> > object ID for all graph objects, and have some shared fields
> > that will be common on all media graph objects.
> >
> > Right now, the only common object is the object ID, but other
> > fields will be added later on.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index cb0ac4e0dfa5..4834172bf6f8 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -27,6 +27,38 @@
> > #include <media/media-device.h>
> >
> > /**
> > + * media_gobj_init - Initialize a graph object
> > + *
> > + * @mdev: Pointer to the media_device that contains the object
> > + * @type: Type of the object
> > + * @gobj: Pointer to the object
> > + *
> > + * This routine initializes the embedded struct media_gobj inside a
> > + * media graph object. It is called automatically if media_*_create()
> > + * calls are used. However, if the object (entity, link, pad, interface)
> > + * is embedded on some other object, this function should be called before
> > + * registering the object at the media controller.
> > + */
> > +void media_gobj_init(struct media_device *mdev,
> > + enum media_gobj_type type,
> > + struct media_gobj *gobj)
> > +{
> > + /* For now, nothing to do */
> > +}
> > +
> > +/**
> > + * media_gobj_remove - Stop using a graph object on a media device
> > + *
> > + * @graph_obj: Pointer to the object
> > + *
> > + * This should be called at media_device_unregister_*() routines
> > + */
> > +void media_gobj_remove(struct media_gobj *gobj)
> > +{
> > + /* For now, nothing to do */
> > +}
> > +
> > +/**
> > * media_entity_init - Initialize a media entity
> > *
> > * @num_pads: Total number of sink and source pads.
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 0a66fc225559..762593c7424f 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -28,6 +28,37 @@
> > #include <linux/list.h>
> > #include <linux/media.h>
> >
> > +/* Enums used internally at the media controller to represent graphs */
> > +
> > +/**
> > + * enum media_gobj_type - type of a graph element
> > + *
> > + */
> > +enum media_gobj_type {
> > + /* FIXME: add the types here, as we embed media_gobj */
> > + MEDIA_GRAPH_NONE
> > +};
> > +
> > +#define BITS_PER_TYPE 8
> > +#define BITS_PER_LOCAL_ID (32 - BITS_PER_TYPE)
>
> I think that these two defines should be prefixed with MEDIA_.
> They are a bit too generic for a header that's going to be used in lots
> of drivers.
> I also think that the local ID mask should be a define as well:
>
> #define MEDIA_LOCAL_ID_MASK GENMASK(BITS_PER_LOCAL_ID - 1, 0)
>
> That will make the code where this mask is used a bit more readable.
Ok, I'll change them.
>
> > +
> > +/* Structs to represent the objects that belong to a media graph */
> > +
> > +/**
> > + * struct media_gobj - Define a graph object.
> > + *
> > + * @id: Non-zero object ID identifier. The ID should be unique
> > + * inside a media_device, as it is composed by
> > + * BITS_PER_TYPE to store the type plus BITS_PER_LOCAL_ID
> > + * to store a per-type ID (called as "local ID").
> > + *
> > + * All elements on the media graph should have this struct embedded
> > + */
> > +struct media_gobj {
> > + u32 id;
> > +};
> > +
> > +
> > struct media_pipeline {
> > };
> >
> > @@ -118,6 +149,26 @@ static inline u32 media_entity_id(struct media_entity *entity)
> > return entity->id;
> > }
> >
> > +static inline enum media_gobj_type media_type(struct media_gobj *gobj)
> > +{
> > + return gobj->id >> BITS_PER_LOCAL_ID;
> > +}
> > +
> > +static inline u32 media_localid(struct media_gobj *gobj)
> > +{
> > + return gobj->id & GENMASK(BITS_PER_LOCAL_ID - 1, 0);
> > +}
> > +
> > +static inline u32 media_gobj_gen_id(enum media_gobj_type type, u32 local_id)
> > +{
> > + u32 id;
> > +
> > + id = type << BITS_PER_LOCAL_ID;
> > + id |= GENMASK(BITS_PER_LOCAL_ID - 1, 0) & local_id;
>
> I'd swap this: id |= local_id & MEDIA_LOCAL_ID_MASK;
>
> It feels more natural that way and it is consistent with media_localid() above.
Ok.
>
> Regards,
>
> Hans
>
> > +
> > + return id;
> > +}
> > +
> > #define MEDIA_ENTITY_ENUM_MAX_DEPTH 16
> > #define MEDIA_ENTITY_ENUM_MAX_ID 64
> >
> > @@ -131,6 +182,14 @@ struct media_entity_graph {
> > int top;
> > };
> >
> > +#define gobj_to_entity(gobj) \
> > + container_of(gobj, struct media_entity, graph_obj)
> > +
> > +void media_gobj_init(struct media_device *mdev,
> > + enum media_gobj_type type,
> > + struct media_gobj *gobj);
> > +void media_gobj_remove(struct media_gobj *gobj);
> > +
> > int media_entity_init(struct media_entity *entity, u16 num_pads,
> > struct media_pad *pads);
> > void media_entity_cleanup(struct media_entity *entity);
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal
2015-08-19 8:36 ` Hans Verkuil
@ 2015-08-19 9:50 ` Mauro Carvalho Chehab
2015-08-19 11:01 ` Hans Verkuil
0 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-19 9:50 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Em Wed, 19 Aug 2015 10:36:51 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
> > It helps to check if the media controller is doing the
> > right thing with the object creation and removal.
> >
> > No extra code/data will be produced if DEBUG or
> > CONFIG_DYNAMIC_DEBUG is not enabled.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index a6be50e04736..9e5ebc83ff76 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -27,6 +27,67 @@
> > #include <media/media-device.h>
> >
> > /**
> > + * dev_dbg_obj - Prints in debug mode a change on some object
> > + *
> > + * @event_name: Name of the event to report. Could be __func__
> > + * @gobj: Pointer to the object
> > + *
> > + * Enabled only if DEBUG or CONFIG_DYNAMIC_DEBUG. Otherwise, it
> > + * won't produce any code.
> > + */
> > +static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
> > +{
> > +#if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
> > + static const char *gobj_type[] = {
> > + [MEDIA_GRAPH_ENTITY] = "entity",
> > + [MEDIA_GRAPH_PAD] = "pad",
> > + [MEDIA_GRAPH_LINK] = "link",
> > + };
> > +
> > + switch (media_type(gobj)) {
> > + case MEDIA_GRAPH_ENTITY:
> > + dev_dbg(gobj->mdev->dev,
> > + "%s: id 0x%08x %s#%d '%s'\n",
> > + event_name, gobj->id,
> > + gobj_type[media_type(gobj)],
>
> You can just fill in "entity" in the string since this is always an entity in
> this case.
Yes, I know that we actually don't need to use gobj_type[] on the
macros anymore. On my first version, there was no switch(), and just
a generic function. I would prefer this way, though. We'll end by
using the arrays for the links.
So, I'll do such cleanup when adding interfaces and interface links.
>
> > + media_localid(gobj),
> > + gobj_to_entity(gobj)->name);
> > + break;
> > + case MEDIA_GRAPH_LINK:
> > + {
> > + struct media_link *link = gobj_to_link(gobj);
> > +
> > + dev_dbg(gobj->mdev->dev,
> > + "%s: id 0x%08x %s#%d: '%s' %s#%d ==> '%s' %s#%d\n",
> > + event_name, gobj->id,
> > + gobj_type[media_type(gobj)],
>
> Ditto.
>
> > + media_localid(gobj),
> > +
> > + link->source->entity->name,
> > + gobj_type[media_type(&link->source->graph_obj)],
> > + media_localid(&link->source->graph_obj),
> > +
> > + link->sink->entity->name,
> > + gobj_type[media_type(&link->sink->graph_obj)],
> > + media_localid(&link->sink->graph_obj));
> > + break;
> > + }
> > + case MEDIA_GRAPH_PAD:
> > + {
> > + struct media_pad *pad = gobj_to_pad(gobj);
> > +
> > + dev_dbg(gobj->mdev->dev,
> > + "%s: id 0x%08x %s %s#%d\n",
> > + event_name, gobj->id,
> > + pad->entity->name,
> > + gobj_type[media_type(gobj)],
>
> Ditto.
>
> > + media_localid(gobj));
> > + }
>
> I would add a default case here with a WARN(1) or something like that to make
> it very clear that someone forgot to extend this switch after adding a new
> object type.
Well, newer gcc versions will print a warning if we add a new type at
the enum and we forget to add here. So, I guess a default case is an
overkill, and will just add extra code that will never be used.
> In the LINK case there is no check for unknown object types in the gobj_type[]
> lookups. I think it is overkill to add a check for that since the objects that
> are linked are going to be created first, so they would trigger the WARN(1) in
> the default case first if they are using unhandled types. That should give
> enough information for the developer.
Well, I can convert gobj_type into a function and add a default case
there, as going past the array is something that makes sense to avoid.
I'll do such change when writing the support for interface links.
>
> > + }
> > +#endif
> > +}
> > +
> > +/**
> > * media_gobj_init - Initialize a graph object
> > *
> > * @mdev: Pointer to the media_device that contains the object
> > @@ -43,6 +104,8 @@ void media_gobj_init(struct media_device *mdev,
> > enum media_gobj_type type,
> > struct media_gobj *gobj)
> > {
> > + gobj->mdev = mdev;
> > +
> > /* Create a per-type unique object ID */
> > switch (type) {
> > case MEDIA_GRAPH_ENTITY:
> > @@ -55,6 +118,7 @@ void media_gobj_init(struct media_device *mdev,
> > gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
> > break;
> > }
> > + dev_dbg_obj(__func__, gobj);
> > }
> >
> > /**
> > @@ -66,7 +130,7 @@ void media_gobj_init(struct media_device *mdev,
> > */
> > void media_gobj_remove(struct media_gobj *gobj)
> > {
> > - /* For now, nothing to do */
> > + dev_dbg_obj(__func__, gobj);
> > }
> >
> > /**
> > @@ -472,13 +536,6 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
> > entity->links = links;
> > }
> >
> > - /* Initialize graph object embedded at the new link */
> > - if (entity->parent)
> > - media_gobj_init(entity->parent, MEDIA_GRAPH_LINK,
> > - &entity->links[entity->num_links].graph_obj);
> > - else
> > - pr_warn("Link created before entity register!\n");
> > -
>
> This is confusing: this was added in patch 5 and now it is removed again. This doesn't
> seem to have anything to do with adding this debugging facility.
The warning is a left over from the test phase. I was afraid that such
condition might be happening.
With regards to media_gobj_init(), we can't call it here anymore, as
it is too early to call dev_dbg_obj(), as the link data is not filled.
So, this got moved to media_entity_create_link(), to happen after
filling link->source and link->sink.
>
> > return &entity->links[entity->num_links++];
> > }
> >
> > @@ -501,6 +558,8 @@ )struct media_entity *source, u16 source_pad,
> > link->sink = &sink->pads[sink_pad];
> > link->flags = flags;
> >
> > + media_gobj_init(source->parent, MEDIA_GRAPH_LINK, &link->graph_obj);
> > +
>
> Same for this,
>
> > /* Create the backlink. Backlinks are used to help graph traversal and
> > * are not reported to userspace.
> > */
> > @@ -514,6 +573,8 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
> > backlink->sink = &sink->pads[sink_pad];
> > backlink->flags = flags;
> >
> > + media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
> > +
>
> and this.
>
> Shouldn't these three changes be moved to patch 5?
Well, I can move this hunk to patch 5, but that will just be hiding
the reason why they should be here in the first place: due to the new
debug function.
>
> > link->reverse = backlink;
> > backlink->reverse = link;
> >
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 2ffe015629fa..db1f54f09723 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -59,6 +59,7 @@ enum media_gobj_type {
> > * All elements on the media graph should have this struct embedded
> > */
> > struct media_gobj {
> > + struct media_device *mdev;
>
> If you add this here, then the parent pointer in struct media_entity should be
> removed to avoid having duplicate fields.
Yes, I know, but I don't want to do it on just one patch. I'm trying to
do the minimal amount of changes to introduce interface and interface
links. My plan is to actually remove entity->parent latter, as such change
will touch inside the drivers. It could be a big patch that won't cause
any real gain, except for 4 bytes saved. So, I really prefer to postpone
this to a cleanup series. I'm also hoping to merge vimc driver before that,
as this would mean one less thing to be rebased on it ;)
>
> I understand why you add this and I don't object, but I would split this change
> off into a separate patch that also includes removing the media_entity parent
> field.
Well, you didn't complain about that on the previous versions (and mdev
were on all other patches) :-)
I actually think that this way is easier to review, as one of the usages
of mdev is explicit on this patch, and, as I said, the removal of the
entity->parent is just a cleanup that can be done latter.
Let's focus on adding the new functionality first.
>
> Regards,
>
> Hans
>
> > u32 id;
> > };
> >
> > @@ -190,6 +191,12 @@ struct media_entity_graph {
> > #define gobj_to_entity(gobj) \
> > container_of(gobj, struct media_entity, graph_obj)
> >
> > +#define gobj_to_pad(gobj) \
> > + container_of(gobj, struct media_pad, graph_obj)
> > +
> > +#define gobj_to_link(gobj) \
> > + container_of(gobj, struct media_link, graph_obj)
> > +
> > void media_gobj_init(struct media_device *mdev,
> > enum media_gobj_type type,
> > struct media_gobj *gobj);
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal
2015-08-19 9:50 ` Mauro Carvalho Chehab
@ 2015-08-19 11:01 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2015-08-19 11:01 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
On 08/19/15 11:50, Mauro Carvalho Chehab wrote:
> Em Wed, 19 Aug 2015 10:36:51 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
>> On 08/18/2015 10:04 PM, Mauro Carvalho Chehab wrote:
>>> It helps to check if the media controller is doing the
>>> right thing with the object creation and removal.
>>>
>>> No extra code/data will be produced if DEBUG or
>>> CONFIG_DYNAMIC_DEBUG is not enabled.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>
>>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>>> index a6be50e04736..9e5ebc83ff76 100644
>>> --- a/drivers/media/media-entity.c
>>> +++ b/drivers/media/media-entity.c
>>> @@ -27,6 +27,67 @@
>>> #include <media/media-device.h>
>>>
>>> /**
>>> + * dev_dbg_obj - Prints in debug mode a change on some object
>>> + *
>>> + * @event_name: Name of the event to report. Could be __func__
>>> + * @gobj: Pointer to the object
>>> + *
>>> + * Enabled only if DEBUG or CONFIG_DYNAMIC_DEBUG. Otherwise, it
>>> + * won't produce any code.
>>> + */
>>> +static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
>>> +{
>>> +#if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
>>> + static const char *gobj_type[] = {
>>> + [MEDIA_GRAPH_ENTITY] = "entity",
>>> + [MEDIA_GRAPH_PAD] = "pad",
>>> + [MEDIA_GRAPH_LINK] = "link",
>>> + };
>>> +
>>> + switch (media_type(gobj)) {
>>> + case MEDIA_GRAPH_ENTITY:
>>> + dev_dbg(gobj->mdev->dev,
>>> + "%s: id 0x%08x %s#%d '%s'\n",
>>> + event_name, gobj->id,
>>> + gobj_type[media_type(gobj)],
>>
>> You can just fill in "entity" in the string since this is always an entity in
>> this case.
>
> Yes, I know that we actually don't need to use gobj_type[] on the
> macros anymore. On my first version, there was no switch(), and just
> a generic function. I would prefer this way, though. We'll end by
> using the arrays for the links.
>
> So, I'll do such cleanup when adding interfaces and interface links.
OK.
>
>>
>>> + media_localid(gobj),
>>> + gobj_to_entity(gobj)->name);
>>> + break;
>>> + case MEDIA_GRAPH_LINK:
>>> + {
>>> + struct media_link *link = gobj_to_link(gobj);
>>> +
>>> + dev_dbg(gobj->mdev->dev,
>>> + "%s: id 0x%08x %s#%d: '%s' %s#%d ==> '%s' %s#%d\n",
>>> + event_name, gobj->id,
>>> + gobj_type[media_type(gobj)],
>>
>> Ditto.
>>
>>> + media_localid(gobj),
>>> +
>>> + link->source->entity->name,
>>> + gobj_type[media_type(&link->source->graph_obj)],
>>> + media_localid(&link->source->graph_obj),
>>> +
>>> + link->sink->entity->name,
>>> + gobj_type[media_type(&link->sink->graph_obj)],
>>> + media_localid(&link->sink->graph_obj));
>>> + break;
>>> + }
>>> + case MEDIA_GRAPH_PAD:
>>> + {
>>> + struct media_pad *pad = gobj_to_pad(gobj);
>>> +
>>> + dev_dbg(gobj->mdev->dev,
>>> + "%s: id 0x%08x %s %s#%d\n",
>>> + event_name, gobj->id,
>>> + pad->entity->name,
>>> + gobj_type[media_type(gobj)],
>>
>> Ditto.
>>
>>> + media_localid(gobj));
>>> + }
>>
>> I would add a default case here with a WARN(1) or something like that to make
>> it very clear that someone forgot to extend this switch after adding a new
>> object type.
>
> Well, newer gcc versions will print a warning if we add a new type at
> the enum and we forget to add here. So, I guess a default case is an
> overkill, and will just add extra code that will never be used.
Ah, that's true.
>
>> In the LINK case there is no check for unknown object types in the gobj_type[]
>> lookups. I think it is overkill to add a check for that since the objects that
>> are linked are going to be created first, so they would trigger the WARN(1) in
>> the default case first if they are using unhandled types. That should give
>> enough information for the developer.
>
> Well, I can convert gobj_type into a function and add a default case
> there, as going past the array is something that makes sense to avoid.
>
> I'll do such change when writing the support for interface links.
OK.
>
>>
>>> + }
>>> +#endif
>>> +}
>>> +
>>> +/**
>>> * media_gobj_init - Initialize a graph object
>>> *
>>> * @mdev: Pointer to the media_device that contains the object
>>> @@ -43,6 +104,8 @@ void media_gobj_init(struct media_device *mdev,
>>> enum media_gobj_type type,
>>> struct media_gobj *gobj)
>>> {
>>> + gobj->mdev = mdev;
>>> +
>>> /* Create a per-type unique object ID */
>>> switch (type) {
>>> case MEDIA_GRAPH_ENTITY:
>>> @@ -55,6 +118,7 @@ void media_gobj_init(struct media_device *mdev,
>>> gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
>>> break;
>>> }
>>> + dev_dbg_obj(__func__, gobj);
>>> }
>>>
>>> /**
>>> @@ -66,7 +130,7 @@ void media_gobj_init(struct media_device *mdev,
>>> */
>>> void media_gobj_remove(struct media_gobj *gobj)
>>> {
>>> - /* For now, nothing to do */
>>> + dev_dbg_obj(__func__, gobj);
>>> }
>>>
>>> /**
>>> @@ -472,13 +536,6 @@ static struct media_link *media_entity_add_link(struct media_entity *entity)
>>> entity->links = links;
>>> }
>>>
>>> - /* Initialize graph object embedded at the new link */
>>> - if (entity->parent)
>>> - media_gobj_init(entity->parent, MEDIA_GRAPH_LINK,
>>> - &entity->links[entity->num_links].graph_obj);
>>> - else
>>> - pr_warn("Link created before entity register!\n");
>>> -
>>
>> This is confusing: this was added in patch 5 and now it is removed again. This doesn't
>> seem to have anything to do with adding this debugging facility.
>
> The warning is a left over from the test phase. I was afraid that such
> condition might be happening.
>
> With regards to media_gobj_init(), we can't call it here anymore, as
> it is too early to call dev_dbg_obj(), as the link data is not filled.
> So, this got moved to media_entity_create_link(), to happen after
> filling link->source and link->sink.
>
>>
>>> return &entity->links[entity->num_links++];
>>> }
>>>
>>> @@ -501,6 +558,8 @@ )struct media_entity *source, u16 source_pad,
>>> link->sink = &sink->pads[sink_pad];
>>> link->flags = flags;
>>>
>>> + media_gobj_init(source->parent, MEDIA_GRAPH_LINK, &link->graph_obj);
>>> +
>>
>> Same for this,
>>
>>> /* Create the backlink. Backlinks are used to help graph traversal and
>>> * are not reported to userspace.
>>> */
>>> @@ -514,6 +573,8 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
>>> backlink->sink = &sink->pads[sink_pad];
>>> backlink->flags = flags;
>>>
>>> + media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
>>> +
>>
>> and this.
>>
>> Shouldn't these three changes be moved to patch 5?
>
> Well, I can move this hunk to patch 5, but that will just be hiding
> the reason why they should be here in the first place: due to the new
> debug function.
As a reviewer it is confusing when something is added in one patch and promptly
removed again in the next. Just saying...
>
>>
>>> link->reverse = backlink;
>>> backlink->reverse = link;
>>>
>>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>>> index 2ffe015629fa..db1f54f09723 100644
>>> --- a/include/media/media-entity.h
>>> +++ b/include/media/media-entity.h
>>> @@ -59,6 +59,7 @@ enum media_gobj_type {
>>> * All elements on the media graph should have this struct embedded
>>> */
>>> struct media_gobj {
>>> + struct media_device *mdev;
>>
>> If you add this here, then the parent pointer in struct media_entity should be
>> removed to avoid having duplicate fields.
>
> Yes, I know, but I don't want to do it on just one patch. I'm trying to
> do the minimal amount of changes to introduce interface and interface
> links. My plan is to actually remove entity->parent latter, as such change
> will touch inside the drivers. It could be a big patch that won't cause
> any real gain, except for 4 bytes saved. So, I really prefer to postpone
> this to a cleanup series. I'm also hoping to merge vimc driver before that,
> as this would mean one less thing to be rebased on it ;)
OK, but then please add a comment before the parent field that mentions that
it is now a duplicate of mdev and will be removed in a future patch (or
something along those lines).
Regards,
Hans
>
>>
>> I understand why you add this and I don't object, but I would split this change
>> off into a separate patch that also includes removing the media_entity parent
>> field.
>
> Well, you didn't complain about that on the previous versions (and mdev
> were on all other patches) :-)
>
> I actually think that this way is easier to review, as one of the usages
> of mdev is explicit on this patch, and, as I said, the removal of the
> entity->parent is just a cleanup that can be done latter.
>
> Let's focus on adding the new functionality first.
>
>>
>> Regards,
>>
>> Hans
>>
>>> u32 id;
>>> };
>>>
>>> @@ -190,6 +191,12 @@ struct media_entity_graph {
>>> #define gobj_to_entity(gobj) \
>>> container_of(gobj, struct media_entity, graph_obj)
>>>
>>> +#define gobj_to_pad(gobj) \
>>> + container_of(gobj, struct media_pad, graph_obj)
>>> +
>>> +#define gobj_to_link(gobj) \
>>> + container_of(gobj, struct media_link, graph_obj)
>>> +
>>> void media_gobj_init(struct media_device *mdev,
>>> enum media_gobj_type type,
>>> struct media_gobj *gobj);
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-08-19 11:04 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-18 20:04 [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 1/8] [media] media: create a macro to get entity ID Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 2/8] [media] media: add a common struct to be embed on media graph objects Mauro Carvalho Chehab
2015-08-19 8:17 ` Hans Verkuil
2015-08-19 9:19 ` Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 3/8] [media] media: use media_gobj inside entities Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 4/8] [media] media: use media_gobj inside pads Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 5/8] [media] media: use media_gobj inside links Mauro Carvalho Chehab
2015-08-19 8:18 ` Hans Verkuil
2015-08-18 20:04 ` [PATCH RFC v5 6/8] [media] media: add messages when media device gets (un)registered Mauro Carvalho Chehab
2015-08-18 20:04 ` [PATCH RFC v5 7/8] [media] media: add a debug message to warn about gobj creation/removal Mauro Carvalho Chehab
2015-08-19 8:36 ` Hans Verkuil
2015-08-19 9:50 ` Mauro Carvalho Chehab
2015-08-19 11:01 ` Hans Verkuil
2015-08-18 20:04 ` [PATCH RFC v5 8/8] [media] media: rename the function that create pad links Mauro Carvalho Chehab
2015-08-19 8:38 ` Hans Verkuil
2015-08-18 20:08 ` [PATCH RFC v5 0/8] MC preparation patches Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).