Re: [media-workshop] [PATCH v8.4 14/83] [media] media: add functions to allow creating interfaces

Em Thu, 29 Oct 2015 11:49:46 +0200 Sakari Ailus sakari.ailus@iki.fi escreveu:

Hi Mauro,

On Mon, Oct 12, 2015 at 01:43:03PM -0300, Mauro Carvalho Chehab wrote:

...

Interfaces are different than entities: they represent a Kernel<->userspace interaction, while entities represent a piece of hardware/firmware/software that executes a function.

Let's distinguish them by creating a separate structure to store the interfaces.

Later patches should change the existing drivers and logic to split the current interface embedded inside the entity structure (device nodes) into a separate object of the graph.

Change-Id: I0c0cb096e74d829ebd342dbaa54413524ea3b2ae Signed-off-by: Mauro Carvalho Chehab mchehab@osg.samsung.com

---

drivers/media/media-entity.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ include/media/media-device.h | 2 ++ include/media/media-entity.h | 48 +++++++++++++++++++++++++ 3 files changed, 133 insertions(+)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c index a23c93369a04..dc679dfe8ade 100644 --- a/drivers/media/media-entity.c +++ b/drivers/media/media-entity.c @@ -44,11 +44,41 @@ static inline const char *gobj_type(enum media_gobj_type type) return "pad"; case MEDIA_GRAPH_LINK: return "link";

• case MEDIA_GRAPH_INTF_DEVNODE:

• return "intf-devnode";
  

  default: return "unknown"; }

}

+static inline const char *intf_type(struct media_interface *intf) +{

• switch (intf->type) {
• case MEDIA_INTF_T_DVB_FE:

• return "frontend";
  

• case MEDIA_INTF_T_DVB_DEMUX:

• return "demux";
  

• case MEDIA_INTF_T_DVB_DVR:

• return "DVR";
  

• case MEDIA_INTF_T_DVB_CA:

• return  "CA";
  

• case MEDIA_INTF_T_DVB_NET:

• return "dvbnet";
  

• case MEDIA_INTF_T_V4L_VIDEO:

• return "video";
  

• case MEDIA_INTF_T_V4L_VBI:

• return "vbi";
  

• case MEDIA_INTF_T_V4L_RADIO:

• return "radio";
  

• case MEDIA_INTF_T_V4L_SUBDEV:

• return "v4l2-subdev";
  

• case MEDIA_INTF_T_V4L_SWRADIO:

• return "swradio";
  

• default:

• return "unknown-intf";
  

• }

+};

static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj) { #if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG) @@ -84,6 +114,19 @@ static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj) "%s: id 0x%08x pad#%d: '%s':%d\n", event_name, gobj->id, media_localid(gobj), pad->entity->name, pad->index);

• break;
  

• }
• case MEDIA_GRAPH_INTF_DEVNODE:
• {

• struct media_interface *intf = gobj_to_intf(gobj);
  

• struct media_intf_devnode *devnode = intf_to_devnode(intf);
  

• dev_dbg(gobj->mdev->dev,
  

• 	"%s: id 0x%08x intf_devnode#%d: %s - major: %d, minor: %d\n",
  

• 	event_name, gobj->id, media_localid(gobj),
  

• 	intf_type(intf),
  

• 	devnode->major, devnode->minor);
  

• break;
  

  } }

#endif @@ -119,6 +162,9 @@ void media_gobj_init(struct media_device *mdev, case MEDIA_GRAPH_LINK: gobj->id = media_gobj_gen_id(type, ++mdev->link_id); break;

• case MEDIA_GRAPH_INTF_DEVNODE:

• gobj->id = media_gobj_gen_id(type, ++mdev->intf_devnode_id);
  

• break;
  

  } dev_dbg_obj(__func__, gobj);

} @@ -793,3 +839,40 @@ struct media_pad *media_entity_remote_pad(struct media_pad *pad)

} EXPORT_SYMBOL_GPL(media_entity_remote_pad);

+/* Functions related to the media interface via device nodes */

+struct media_intf_devnode *media_devnode_create(struct media_device *mdev,

• 				u32 type, u32 flags,
  

• 				u32 major, u32 minor,
  

• 				gfp_t gfp_flags)
  

+{

• struct media_intf_devnode *devnode;
• struct media_interface *intf;
• devnode = kzalloc(sizeof(*devnode), gfp_flags);
• if (!devnode)

• return NULL;
  

• intf = &devnode->intf;
• intf->type = type;
• intf->flags = flags;
• devnode->major = major;
• devnode->minor = minor;
• media_gobj_init(mdev, MEDIA_GRAPH_INTF_DEVNODE,

•        &devnode->intf.graph_obj);
  

• return devnode;

+} +EXPORT_SYMBOL_GPL(media_devnode_create);

+void media_devnode_remove(struct media_intf_devnode *devnode) +{

• media_gobj_remove(&devnode->intf.graph_obj);
• kfree(devnode);

+} +EXPORT_SYMBOL_GPL(media_devnode_remove); diff --git a/include/media/media-device.h b/include/media/media-device.h index 05414e351f8e..3b14394d5701 100644 --- a/include/media/media-device.h +++ b/include/media/media-device.h @@ -44,6 +44,7 @@ struct device;

• @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

• • @intf_devnode_id: Unique ID used on the last interface devnode registered
  • @entities: List of registered entities
  • @lock: Entities list lock
  • @graph_mutex: Entities graph operation lock

@@ -73,6 +74,7 @@ struct media_device { u32 entity_id; u32 pad_id; u32 link_id;

• u32 intf_devnode_id;

  struct list_head entities;

diff --git a/include/media/media-entity.h b/include/media/media-entity.h index 239c4ec30ef6..7df8836f4eef 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -36,11 +36,14 @@

• @MEDIA_GRAPH_ENTITY: Identify a media entity
• @MEDIA_GRAPH_PAD: Identify a media pad
• @MEDIA_GRAPH_LINK: Identify a media link

• • @MEDIA_GRAPH_INTF_DEVNODE: Identify a media Kernel API interface via

• • 		a device node
    

  */

enum media_gobj_type { MEDIA_GRAPH_ENTITY, MEDIA_GRAPH_PAD, MEDIA_GRAPH_LINK,

• MEDIA_GRAPH_INTF_DEVNODE,

};

#define MEDIA_BITS_PER_TYPE 8 @@ -141,6 +144,34 @@ struct media_entity { } info; };

+/**

• • struct media_intf_devnode - Define a Kernel API interface
• • @graph_obj: embedded graph object
• • @type: Type of the interface as defined at the

• • 	uapi/media/media.h header, e. g.
    

• • 	MEDIA_INTF_T_*
    

• • @flags: Interface flags as defined at uapi/media/media.h
• */

+struct media_interface {

• struct media_gobj graph_obj;
• u32 type;
• u32 flags;

+};

+/**

• • struct media_intf_devnode - Define a Kernel API interface via a device node
• • @intf: embedded interface object
• • @major: Major number of a device node
• • @minor: Minor number of a device node
• */

+struct media_intf_devnode {

• struct media_interface intf;
• u32 major;
• u32 minor;

+};

static inline u32 media_entity_type(struct media_entity *entity) { return entity->type & MEDIA_ENT_TYPE_MASK; @@ -198,6 +229,18 @@ struct media_entity_graph { #define gobj_to_link(gobj) \ container_of(gobj, struct media_link, graph_obj)

+#define gobj_to_link(gobj) \

• container_of(gobj, struct media_link, graph_obj)
  

+#define gobj_to_pad(gobj) \

• container_of(gobj, struct media_pad, graph_obj)
  

+#define gobj_to_intf(gobj) \

• container_of(gobj, struct media_interface, graph_obj)
  

+#define intf_to_devnode(intf) \

• container_of(intf, struct media_intf_devnode, intf)
  

void media_gobj_init(struct media_device *mdev, enum media_gobj_type type, struct media_gobj *gobj); @@ -229,6 +272,11 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity, struct media_pipeline *pipe); void media_entity_pipeline_stop(struct media_entity *entity);

+struct media_intf_devnode *media_devnode_create(struct media_device *mdev,

• 				u32 type, u32 flags,
  

• 				u32 major, u32 minor,
  

• 				gfp_t gfp_flags);
  

+void media_devnode_remove(struct media_intf_devnode *devnode); #define media_entity_call(entity, operation, args...) \ (((entity)->ops && (entity)->ops->operation) ? \ (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)

I'm beginning to feel we should probably reconsider the names of the headers and what goes to which. What do you think? I'm totally fine postponing that after the patchset, though.

I agree: "media-entity.[ch]" actually a bad name, as it actually has functions for all graph types. Perhaps this could be renamed to "media-graph.[ch]", and maybe "media-device.[ch]" to "media-ioctl.[ch]", eventually moving some functions between those files/headers.

I would do this on some latter patch series.

Do we have a to-do list of some kind btw.?

Yes, I started writing one during version 8.3. I'm pretty sure that it is not complete.

Also, some items were already accomplished. For example, I merged Javier's patches on version 8.4. I'll double check the TODO list next week, after getting Laurent's review.

I'm enclosing what I wrote on that time.

Regards, Mauro

-----------------

TODO for next Kernel version (goal: Kernel version 4.4): =========================================================

- Add Javier's fixup patches with fixes for some platform drivers and uvc;

- Find entities that belong to V4L2 or DVB via the interfaces, in order to enable/disable the inteface links when the device gets busy;

TODO for a next versions: =========================

- Remove unused fields from media_entity (like major, minor, revision, group_id, num_links, num_backlinks, num_pads)

- dynamic entity/interface/link creation and removal;

- SETUP_LINK_V2 with dynamic support;

- dynamic pad creation and removal (needed?);

- multiple function per entity support;

- indirect interface links support;

- MC properties API.

Userspace: ==========

- Create a library with v2 API;

- Use the v2 API library on qv4l2/libdvbv5/xawtv/libv4l;

- Add the libudev/libsysfs logic at mc_nextgen_test to convert a devnode major/minor into a /dev/* name;

Other Nice things =================

- Rename media_entity_init() to media_entity_create_pads() - Rename media_graph_init() to media_graph_register()