[PATCH v8 00/55] MC next generation patches

That's the 8th version of the MC next generation patches.

Differences from version 7:

- Patches reworked to make the reviewers happy;
- Bug fixes;
- ALSA changes got their own separate patches;
- Javier patches got integrated into this series;
- media-entity.h structs are now properly documented;
- Tested on both au0828 and omap3isp.

Due to the complexity of this change, other platform drivers may
require some fixes. 

As the patch series sent before, this is not meant to be sent
upstream yet. Its goal is to merge it for Kernel 4.4, in order to
give people enough time to review and fix pending issues.

Regards,
Mauro

Javier Martinez Canillas (6):
  [media] staging: omap4iss: get entity ID using media_entity_id()
  [media] omap3isp: get entity ID using media_entity_id()
  [media] media: use entity.graph_obj.mdev instead of .parent
  [media] media: remove media entity .parent field
  [media] omap3isp: separate links creation from entities init
  [media] omap3isp: create links after all subdevs have been bound

Mauro Carvalho Chehab (49):
  [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
  [media] uapi/media.h: Declare interface types for V4L2 and DVB
  [media] media: add functions to allow creating interfaces
  [media] uapi/media.h: Declare interface types for ALSA
  [media] media: Don't accept early-created links
  [media] media: convert links from array to list
  [media] media: make add link more generic
  [media] media: make media_link more generic to handle interace links
  [media] media: make link debug printk more generic
  [media] media: add support to link interfaces and entities
  [media] media-entity: add a helper function to create interface
  [media] dvbdev: add support for interfaces
  [media] media: add a linked list to track interfaces by mdev
  [media] dvbdev: add support for indirect interface links
  [media] uapi/media.h: Fix entity namespace
  [media] replace all occurrences of MEDIA_ENT_T_DEVNODE_V4L
  [media] replace all occurrences of MEDIA_ENT_T_DEVNODE_DVB
  [media] media: add macros to check if subdev or V4L2 DMA
  [media] media: use macros to check for V4L2 subdev entities
  [media] omap3/omap4/davinci: get rid of MEDIA_ENT_T_V4L2_SUBDEV abuse
  [media] s5c73m3: fix subdev type
  [media] s5k5baf: fix subdev type
  [media] davinci_vbpe: stop MEDIA_ENT_T_V4L2_SUBDEV abuse
  [media] omap4iss: stop MEDIA_ENT_T_V4L2_SUBDEV abuse
  [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs
  [media] media controller: get rid of entity subtype on Kernel
  [media] media.h: don't use legacy entity macros at Kernel
  [media] DocBook: update descriptions for the media controller entities
  [media] dvb: modify core to implement interfaces/entities at MC new
    gen
  [media] media: report if a pad is sink or source at debug msg
  [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl
  [media] media: Use a macro to interate between all interfaces
  [media] media: move mdev list init to gobj
  [media] media-device: add pads and links to media_device
  [media] media_device: add a topology version field
  [media] media-device: add support for MEDIA_IOC_G_TOPOLOGY ioctl
  [media] media-entity: unregister entity links
  [media] remove interface links at media_entity_unregister()
  [media] media-device: remove interfaces and interface links
  [media] v4l2-core: create MC interfaces for devnodes
  [media] au0828: unregister MC at the end
  [media] media-entity.h: document all the structs

 .../DocBook/media/v4l/media-ioc-enum-entities.xml  |  57 +--
 Documentation/media-framework.txt                  |   2 +-
 drivers/media/dvb-core/dmxdev.c                    |   4 +-
 drivers/media/dvb-core/dvb_ca_en50221.c            |   2 +-
 drivers/media/dvb-core/dvb_frontend.c              |  11 +-
 drivers/media/dvb-core/dvb_net.c                   |   2 +-
 drivers/media/dvb-core/dvbdev.c                    | 278 +++++++++---
 drivers/media/dvb-core/dvbdev.h                    |  10 +-
 drivers/media/firewire/firedtv-ci.c                |   2 +-
 drivers/media/i2c/s5c73m3/s5c73m3-core.c           |   8 +-
 drivers/media/i2c/s5k5baf.c                        |   4 +-
 drivers/media/i2c/smiapp/smiapp-core.c             |   4 +-
 drivers/media/media-device.c                       | 245 +++++++++--
 drivers/media/media-entity.c                       | 473 +++++++++++++++++----
 drivers/media/pci/bt8xx/dst_ca.c                   |   3 +-
 drivers/media/pci/ddbridge/ddbridge-core.c         |   2 +-
 drivers/media/pci/ngene/ngene-core.c               |   2 +-
 drivers/media/pci/ttpci/av7110.c                   |   2 +-
 drivers/media/pci/ttpci/av7110_av.c                |   4 +-
 drivers/media/pci/ttpci/av7110_ca.c                |   2 +-
 drivers/media/platform/exynos4-is/common.c         |   3 +-
 drivers/media/platform/exynos4-is/fimc-capture.c   |   5 +-
 drivers/media/platform/exynos4-is/fimc-isp-video.c |   9 +-
 drivers/media/platform/exynos4-is/fimc-lite.c      |  18 +-
 drivers/media/platform/exynos4-is/media-dev.c      |  25 +-
 drivers/media/platform/exynos4-is/media-dev.h      |   8 +-
 drivers/media/platform/omap3isp/isp.c              | 202 +++++----
 drivers/media/platform/omap3isp/ispccdc.c          |  39 +-
 drivers/media/platform/omap3isp/ispccdc.h          |   1 +
 drivers/media/platform/omap3isp/ispccp2.c          |  35 +-
 drivers/media/platform/omap3isp/ispccp2.h          |   1 +
 drivers/media/platform/omap3isp/ispcsi2.c          |  33 +-
 drivers/media/platform/omap3isp/ispcsi2.h          |   1 +
 drivers/media/platform/omap3isp/isppreview.c       |  48 ++-
 drivers/media/platform/omap3isp/isppreview.h       |   1 +
 drivers/media/platform/omap3isp/ispresizer.c       |  46 +-
 drivers/media/platform/omap3isp/ispresizer.h       |   1 +
 drivers/media/platform/omap3isp/ispvideo.c         |  17 +-
 drivers/media/platform/s3c-camif/camif-capture.c   |   2 +-
 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           |  15 +-
 drivers/media/platform/vsp1/vsp1_wpf.c             |   2 +-
 drivers/media/platform/xilinx/xilinx-dma.c         |  10 +-
 drivers/media/platform/xilinx/xilinx-vipp.c        |   4 +-
 drivers/media/usb/au0828/au0828-core.c             |  18 +-
 drivers/media/usb/au0828/au0828-video.c            |   8 +-
 drivers/media/usb/cx231xx/cx231xx-cards.c          |   6 +-
 drivers/media/usb/cx231xx/cx231xx-video.c          |   8 +-
 drivers/media/usb/uvc/uvc_entity.c                 |   2 +-
 drivers/media/v4l2-core/v4l2-dev.c                 | 105 ++++-
 drivers/media/v4l2-core/v4l2-device.c              |  10 +-
 drivers/media/v4l2-core/v4l2-subdev.c              |   6 +-
 drivers/staging/media/davinci_vpfe/dm365_ipipe.c   |   9 +-
 drivers/staging/media/davinci_vpfe/dm365_ipipeif.c |  15 +-
 drivers/staging/media/davinci_vpfe/dm365_isif.c    |  15 +-
 drivers/staging/media/davinci_vpfe/dm365_resizer.c |  33 +-
 .../staging/media/davinci_vpfe/vpfe_mc_capture.c   |  10 +-
 drivers/staging/media/davinci_vpfe/vpfe_video.c    |  17 +-
 drivers/staging/media/omap4iss/iss.c               |  32 +-
 drivers/staging/media/omap4iss/iss_csi2.c          |  13 +-
 drivers/staging/media/omap4iss/iss_ipipe.c         |   9 +-
 drivers/staging/media/omap4iss/iss_ipipeif.c       |  15 +-
 drivers/staging/media/omap4iss/iss_resizer.c       |  13 +-
 drivers/staging/media/omap4iss/iss_video.c         |   9 +-
 include/media/media-device.h                       |  34 +-
 include/media/media-entity.h                       | 305 +++++++++++--
 include/media/v4l2-dev.h                           |   1 +
 include/uapi/linux/media.h                         | 205 ++++++++-
 70 files changed, 1924 insertions(+), 627 deletions(-)

-- 
2.4.3

Declare the interface types to be used on alsa for the new
G_TOPOLOGY ioctl.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 583666e2cc25..01946baa32d5 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c

@@ -74,6 +74,18 @@ static inline const char *intf_type(struct media_interface *intf)
 		return "v4l2-subdev";
 	case MEDIA_INTF_T_V4L_SWRADIO:
 		return "swradio";
+	case MEDIA_INTF_T_ALSA_PCM_CAPTURE:
+		return "pcm-capture";
+	case MEDIA_INTF_T_ALSA_PCM_PLAYBACK:
+		return "pcm-playback";
+	case MEDIA_INTF_T_ALSA_CONTROL:
+		return "alsa-control";
+	case MEDIA_INTF_T_ALSA_COMPRESS:
+		return "compress";
+	case MEDIA_INTF_T_ALSA_RAWMIDI:
+		return "rawmidi";
+	case MEDIA_INTF_T_ALSA_HWDEP:
+		return "hwdep";
 	default:
 		return "unknown-intf";
 	}

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 3ad3d6be293f..aca828709bad 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -171,6 +171,7 @@ struct media_links_enum {
 
 #define MEDIA_INTF_T_DVB_BASE	0x00000100
 #define MEDIA_INTF_T_V4L_BASE	0x00000200
+#define MEDIA_INTF_T_ALSA_BASE	0x00000300
 
 /* Interface types */
 

@@ -186,6 +187,13 @@ struct media_links_enum {
 #define MEDIA_INTF_T_V4L_SUBDEV (MEDIA_INTF_T_V4L_BASE + 3)
 #define MEDIA_INTF_T_V4L_SWRADIO (MEDIA_INTF_T_V4L_BASE + 4)
 
+#define MEDIA_INTF_T_ALSA_PCM_CAPTURE   (MEDIA_INTF_T_ALSA_BASE)
+#define MEDIA_INTF_T_ALSA_PCM_PLAYBACK  (MEDIA_INTF_T_ALSA_BASE + 1)
+#define MEDIA_INTF_T_ALSA_CONTROL       (MEDIA_INTF_T_ALSA_BASE + 2)
+#define MEDIA_INTF_T_ALSA_COMPRESS      (MEDIA_INTF_T_ALSA_BASE + 3)
+#define MEDIA_INTF_T_ALSA_RAWMIDI       (MEDIA_INTF_T_ALSA_BASE + 4)
+#define MEDIA_INTF_T_ALSA_HWDEP         (MEDIA_INTF_T_ALSA_BASE + 5)
+
 /* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
 
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)

-- 
2.4.3

Put the legacy MEDIA_ENT_* macros under a #ifndef __KERNEL__,
in order to be sure that none of those old symbols are used
inside the Kernel.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index cd486fc25f1e..4186891e5e81 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -107,6 +107,7 @@ struct media_device_info {
 #define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 3)
 #define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 4)
 
+#ifndef __KERNEL__
 /* Legacy symbols used to avoid userspace compilation breakages */
 #define MEDIA_ENT_TYPE_SHIFT		16
 #define MEDIA_ENT_TYPE_MASK		0x00ff0000

@@ -120,6 +121,7 @@ struct media_device_info {
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
 #define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
+#endif
 
 /* Entity types */
 

-- 
2.4.3

Add a new ioctl that will report the entire topology on
one go.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 756e1960fd7f..358a0c6b1f86 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h

@@ -181,6 +181,8 @@ struct media_interface {
  */
 struct media_intf_devnode {
 	struct media_interface		intf;
+
+	/* Should match the fields at media_v2_intf_devnode */
 	u32				major;
 	u32				minor;
 };

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4186891e5e81..fa0b68e670b0 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -251,11 +251,94 @@ struct media_links_enum {
 #define MEDIA_INTF_T_ALSA_RAWMIDI       (MEDIA_INTF_T_ALSA_BASE + 4)
 #define MEDIA_INTF_T_ALSA_HWDEP         (MEDIA_INTF_T_ALSA_BASE + 5)
 
-/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
+/*
+ * MC next gen API definitions
+ *
+ * NOTE: The declarations below are close to the MC RFC for the Media
+ *	 Controller, the next generation. Yet, there are a few adjustments
+ *	 to do, as we want to be able to have a functional API before
+ *	 the MC properties change. Those will be properly marked below.
+ *	 Please also notice that I removed "num_pads", "num_links",
+ *	 from the proposal, as a proper userspace application will likely
+ *	 use lists for pads/links, just as we intend todo in Kernelspace.
+ *	 The API definition should be freed from fields that are bound to
+ *	 some specific data structure.
+ *
+ * FIXME: Currently, I opted to name the new types as "media_v2", as this
+ *	  won't cause any conflict with the Kernelspace namespace, nor with
+ *	  the previous kAPI media_*_desc namespace. This can be changed
+ *	  latter, before the adding this API upstream.
+ */
+
+
+#define MEDIA_NEW_LNK_FL_ENABLED		MEDIA_LNK_FL_ENABLED
+#define MEDIA_NEW_LNK_FL_IMMUTABLE		MEDIA_LNK_FL_IMMUTABLE
+#define MEDIA_NEW_LNK_FL_DYNAMIC		MEDIA_NEW_FL_DYNAMIC
+#define MEDIA_NEW_LNK_FL_INTERFACE_LINK		(1 << 3)
+
+struct media_v2_entity {
+	__u32 id;
+	char name[64];		/* FIXME: move to a property? (RFC says so) */
+	__u16 reserved[14];
+};
+
+/* Should match the specific fields at media_intf_devnode */
+struct media_v2_intf_devnode {
+	__u32 major;
+	__u32 minor;
+};
+
+struct media_v2_interface {
+	__u32 id;
+	__u32 intf_type;
+	__u32 flags;
+	__u32 reserved[9];
+
+	union {
+		struct media_v2_intf_devnode devnode;
+		__u32 raw[16];
+	};
+};
+
+struct media_v2_pad {
+	__u32 id;
+	__u32 entity_id;
+	__u32 flags;
+	__u16 reserved[9];
+};
+
+struct media_v2_link {
+    __u32 id;
+    __u32 source_id;
+    __u32 sink_id;
+    __u32 flags;
+    __u32 reserved[5];
+};
+
+struct media_v2_topology {
+	__u32 topology_version;
+
+	__u32 num_entities;
+	struct media_v2_entity *entities;
+
+	__u32 num_interfaces;
+	struct media_v2_interface *interfaces;
+
+	__u32 num_pads;
+	struct media_v2_pad *pads;
+
+	__u32 num_links;
+	struct media_v2_link *links;
+
+	__u32 reserved[64];
+};
+
+/* ioctls */
 
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)
 #define MEDIA_IOC_SETUP_LINK		_IOWR('|', 0x03, struct media_link_desc)
+#define MEDIA_IOC_G_TOPOLOGY		_IOWR('|', 0x04, struct media_v2_topology)
 
 #endif /* __LINUX_MEDIA_H */

-- 
2.4.3

Now that interfaces got created, we need to fix the entity
namespace.

So, let's create a consistent new namespace and add backward
compatibility macros to keep the old namespace preserved.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 14d32cdcdd47..88013d1a2c39 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c

@@ -230,17 +230,17 @@ static void dvb_create_media_entity(struct dvb_device *dvbdev,
 
 	switch (type) {
 	case DVB_DEVICE_FRONTEND:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DEMUX:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_CA:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;

@@ -439,7 +439,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
 void dvb_create_media_graph(struct dvb_adapter *adap)
 {
 	struct media_device *mdev = adap->mdev;
-	struct media_entity *entity, *tuner = NULL, *fe = NULL;
+	struct media_entity *entity, *tuner = NULL, *demod = NULL;
 	struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
 	struct media_interface *intf;
 

@@ -451,26 +451,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
 		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
 			tuner = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_FE:
-			fe = entity;
+		case MEDIA_ENT_T_DVB_DEMOD:
+			demod = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
+		case MEDIA_ENT_T_DVB_DEMUX:
 			demux = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
+		case MEDIA_ENT_T_DVB_TSOUT:
 			dvr = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_CA:
+		case MEDIA_ENT_T_DVB_CA:
 			ca = entity;
 			break;
 		}
 	}
 
-	if (tuner && fe)
-		media_create_pad_link(tuner, 0, fe, 0, 0);
+	if (tuner && demod)
+		media_create_pad_link(tuner, 0, demod, 0, 0);
 
-	if (fe && demux)
-		media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
+	if (demod && demux)
+		media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
 
 	if (demux && dvr)
 		media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index aca828709bad..3bbda409353f 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -42,31 +42,71 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
+/*
+ * Base numbers for entity types
+ *
+ * Please notice that the huge gap of 16 bits for each base is overkill!
+ * 8 bits is more than enough to avoid starving entity types for each
+ * subsystem.
+ *
+ * However, It is kept this way just to avoid binary breakages with the
+ * namespace provided on legacy versions of this header.
+ */
+#define MEDIA_ENT_T_DVB_BASE		0x00000001
+#define MEDIA_ENT_T_V4L2_BASE		0x00010000
+#define MEDIA_ENT_T_V4L2_SUBDEV_BASE	0x00020000
+
+/*
+ * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
+ *	read()/write() data I/O associated with the V4L2 devnodes.
+ */
+#define MEDIA_ENT_T_V4L2_VIDEO		(MEDIA_ENT_T_V4L2_BASE + 1)
+	/*
+	 * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
+	 * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
+	 * to be declared for FB, ALSA and DVB entities.
+	 * As those values were never actually used in practice, we're just
+	 * adding them as backward compatibility macros and keeping the
+	 * numberspace clean here. This way, we avoid breaking compilation,
+	 * in the case of having some userspace application using the old
+	 * symbols.
+	 */
+#define MEDIA_ENT_T_V4L2_VBI		(MEDIA_ENT_T_V4L2_BASE + 5)
+	/* for TX radio, as RX is done via either ALSA or wire */
+#define MEDIA_ENT_T_V4L2_RADIO		(MEDIA_ENT_T_V4L2_BASE + 6)
+#define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 7)
+
+/* V4L2 Sub-device entities */
+#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
+#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
+#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
+	/* A converter of analogue video to its digital representation. */
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
+	/* Tuner entity is actually both V4L2 and DVB subdev */
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
+
+/* DVB entities */
+#define MEDIA_ENT_T_DVB_DEMOD		(MEDIA_ENT_T_DVB_BASE)
+#define MEDIA_ENT_T_DVB_DEMUX		(MEDIA_ENT_T_DVB_BASE + 1)
+#define MEDIA_ENT_T_DVB_TSOUT		(MEDIA_ENT_T_DVB_BASE + 2)
+#define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 3)
+#define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 4)
+
+/* Legacy symbols used to avoid userspace compilation breakages */
 #define MEDIA_ENT_TYPE_SHIFT		16
 #define MEDIA_ENT_TYPE_MASK		0x00ff0000
 #define MEDIA_ENT_SUBTYPE_MASK		0x0000ffff
 
-#define MEDIA_ENT_T_DEVNODE		(1 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
+#define MEDIA_ENT_T_DEVNODE		MEDIA_ENT_T_V4L2_BASE
+#define MEDIA_ENT_T_V4L2_SUBDEV		MEDIA_ENT_T_V4L2_SUBDEV_BASE
+
+#define MEDIA_ENT_T_DEVNODE_V4L		MEDIA_ENT_T_V4L2_VIDEO
+
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
-#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
-#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
-#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
-#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
+#define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
 
-/* Legacy symbol. Use it to avoid userspace compilation breakages */
-#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
-
-#define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV + 1)
-#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV + 2)
-#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV + 3)
-/* A converter of analogue video to its digital representation. */
-#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
-
-#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
+/* Entity types */
 
 #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
 

-- 
2.4.3

Instead of abusing MEDIA_ENT_T_V4L2_SUBDEV, initialize
new subdev entities as MEDIA_ENT_T_UNKNOWN.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 659507bce63f..134fe7510195 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c

@@ -435,6 +435,12 @@ int __must_check media_device_register_entity(struct media_device *mdev,
 {
 	int i;
 
+	if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN ||
+	    entity->type == MEDIA_ENT_T_UNKNOWN)
+		dev_warn(mdev->dev,
+			 "Entity type for entity %s was not initialized!\n",
+			 entity->name);
+
 	/* Warn if we apparently re-register an entity */
 	WARN_ON(entity->graph_obj.mdev != NULL);
 	entity->graph_obj.mdev = mdev;

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 60da43772de9..b3bcc8253182 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c

@@ -584,7 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
 	sd->host_priv = NULL;
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	sd->entity.name = sd->name;
-	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN;
 #endif
 }
 EXPORT_SYMBOL(v4l2_subdev_init);

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 3bbda409353f..44b84aae8b02 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -42,6 +42,14 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
+/* Used values for media_entity_desc::type */
+
+/*
+ * Initial value when an entity is created
+ * Drivers should change it to something useful
+ */
+#define MEDIA_ENT_T_UNKNOWN	0x00000000
+
 /*
  * Base numbers for entity types
  *

@@ -77,6 +85,15 @@ struct media_device_info {
 #define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 7)
 
 /* V4L2 Sub-device entities */
+
+	/*
+	 * Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
+	 * in order to preserve backward compatibility.
+	 * Drivers should change to the proper subdev type before
+	 * registering the entity.
+	 */
+#define MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN	MEDIA_ENT_T_V4L2_SUBDEV_BASE
+
 #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
 #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
 #define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)

-- 
2.4.3

Declare the interface types that will be used by the new
G_TOPOLOGY ioctl that will be defined latter on.

For now, we need those types, as they'll be used on the
internal structs associated with the new media_interface
graph object defined on the next patch.

Signed-off-by: Mauro Carvalho Chehab <redacted>
Acked-by: Hans Verkuil <redacted>

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4e816be3de39..3ad3d6be293f 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -167,6 +167,27 @@ struct media_links_enum {
 	__u32 reserved[4];
 };
 
+/* Interface type ranges */
+
+#define MEDIA_INTF_T_DVB_BASE	0x00000100
+#define MEDIA_INTF_T_V4L_BASE	0x00000200
+
+/* Interface types */
+
+#define MEDIA_INTF_T_DVB_FE    	(MEDIA_INTF_T_DVB_BASE)
+#define MEDIA_INTF_T_DVB_DEMUX  (MEDIA_INTF_T_DVB_BASE + 1)
+#define MEDIA_INTF_T_DVB_DVR    (MEDIA_INTF_T_DVB_BASE + 2)
+#define MEDIA_INTF_T_DVB_CA     (MEDIA_INTF_T_DVB_BASE + 3)
+#define MEDIA_INTF_T_DVB_NET    (MEDIA_INTF_T_DVB_BASE + 4)
+
+#define MEDIA_INTF_T_V4L_VIDEO  (MEDIA_INTF_T_V4L_BASE)
+#define MEDIA_INTF_T_V4L_VBI    (MEDIA_INTF_T_V4L_BASE + 1)
+#define MEDIA_INTF_T_V4L_RADIO  (MEDIA_INTF_T_V4L_BASE + 2)
+#define MEDIA_INTF_T_V4L_SUBDEV (MEDIA_INTF_T_V4L_BASE + 3)
+#define MEDIA_INTF_T_V4L_SWRADIO (MEDIA_INTF_T_V4L_BASE + 4)
+
+/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
+
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)

-- 
2.4.3

Don't use anymore the type/subtype entity data/macros
inside the Kernel.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index b0cfbc0dffc7..756e1960fd7f 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h

@@ -185,16 +185,6 @@ struct media_intf_devnode {
 	u32				minor;
 };
 
-static inline u32 media_entity_type(struct media_entity *entity)
-{
-	return entity->type & MEDIA_ENT_TYPE_MASK;
-}
-
-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->graph_obj.id;

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 44b84aae8b02..cd486fc25f1e 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -42,10 +42,8 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
-/* Used values for media_entity_desc::type */
-
 /*
- * Initial value when an entity is created
+ * Initial value to be used when a new entity is created
  * Drivers should change it to something useful
  */
 #define MEDIA_ENT_T_UNKNOWN	0x00000000

-- 
2.4.3

Em Sun, 30 Aug 2015 00:06:11 -0300
Mauro Carvalho Chehab [off-list ref] escreveu:

That's the 8th version of the MC next generation patches.

Differences from version 7:

- Patches reworked to make the reviewers happy;
- Bug fixes;
- ALSA changes got their own separate patches;
- Javier patches got integrated into this series;
- media-entity.h structs are now properly documented;
- Tested on both au0828 and omap3isp.

Due to the complexity of this change, other platform drivers may
require some fixes. 

As the patch series sent before, this is not meant to be sent
upstream yet. Its goal is to merge it for Kernel 4.4, in order to
give people enough time to review and fix pending issues.

As on the previous series, the patches are available on my experimental
tree:
	http://git.linuxtv.org/cgit.cgi/mchehab/experimental.git/log/?h=mc_next_gen

There's one additional patch there that removes the backlinks from G_TOPOLOGY
ioctl. I'll be posting it in separate at the ML.

I also added a new branch:
	http://git.linuxtv.org/cgit.cgi/mchehab/experimental.git/log/?h=mc_next_gen_test

Based on the first one. It contains a hack for au0828 that exposes the tuner
also via subdev devnode, and reduces the number of output pads of the DVB
demux to just 5, as the default is too high to produce a .dot file that
would be useful. Of course, this patch should never leave my experimental
tree ;) 

There are a few other from Javier there meant to allow testing the omap3isp
on my Beaglebone (that doesn't have any sensor on it) and on his omap3
devices.

I added support at the mc_nextgen_test tool to produce Graphviz .dot
files. It is still experimental, but it is good enough to already produce
some useful graphs. The newest version is at:

	http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/log/?h=mc-next-gen

I added some graphs produced by it at:
	https://mchehab.fedorapeople.org/mc-next-gen/

Regards,
Mauro

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

I would change this to 0x00000000, see follow-up comment later for why.

+#define MEDIA_ENT_T_V4L2_BASE		0x00010000
+#define MEDIA_ENT_T_V4L2_SUBDEV_BASE	0x00020000
+
+/*
+ * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
+ *	read()/write() data I/O associated with the V4L2 devnodes.
+ */
+#define MEDIA_ENT_T_V4L2_VIDEO		(MEDIA_ENT_T_V4L2_BASE + 1)
+	/*
+	 * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
+	 * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
+	 * to be declared for FB, ALSA and DVB entities.
+	 * As those values were never actually used in practice, we're just
+	 * adding them as backward compatibility macros and keeping the
+	 * numberspace clean here. This way, we avoid breaking compilation,
+	 * in the case of having some userspace application using the old
+	 * symbols.
+	 */
+#define MEDIA_ENT_T_V4L2_VBI		(MEDIA_ENT_T_V4L2_BASE + 5)
+	/* for TX radio, as RX is done via either ALSA or wire */
+#define MEDIA_ENT_T_V4L2_RADIO		(MEDIA_ENT_T_V4L2_BASE + 6)

But TX is also done via either ALSA or wire. This shouldn't be needed.

+#define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 7)

How about MEDIA_ENT_T_DVB_IO_* and MEDIA_ENT_T_V4L2_IO_* to indicate that
this entity deals with data I/O?

Or, perhaps even better, MEDIA_ENT_T_IO_DVB_ and MEDIA_ENT_T_IO_V4L2_.

Entities should do something, and just saying 'V4L2_VIDEO' doesn't really convey
that meaning. It is also very easy to confuse with INTF_T_V4L_* types. BTW, we
should decide whether V4L2 or V4L is used here (interfaces now use V4L, entities
V4L2). Since entities already use V4L2, I think the interface defines should
use V4L2 as well.

+
+/* V4L2 Sub-device entities */
+#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
+#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
+#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
+	/* A converter of analogue video to its digital representation. */
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
+	/* Tuner entity is actually both V4L2 and DVB subdev */
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
+
+/* DVB entities */
+#define MEDIA_ENT_T_DVB_DEMOD		(MEDIA_ENT_T_DVB_BASE)

After changing DVB_BASE to 0, change this to DVB_BASE + 1, and adjust the other DVB
entity types accordingly.

This keeps the base defines consistent (i.e. the lower 16 bits are always 0).

It surprised me when reading this patch, so I'm probably not the only one.

Regards,

	Hans

+#define MEDIA_ENT_T_DVB_DEMUX		(MEDIA_ENT_T_DVB_BASE + 1)
+#define MEDIA_ENT_T_DVB_TSOUT		(MEDIA_ENT_T_DVB_BASE + 2)
+#define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 3)
+#define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 4)
+
+/* Legacy symbols used to avoid userspace compilation breakages */
 #define MEDIA_ENT_TYPE_SHIFT		16
 #define MEDIA_ENT_TYPE_MASK		0x00ff0000
 #define MEDIA_ENT_SUBTYPE_MASK		0x0000ffff
 
-#define MEDIA_ENT_T_DEVNODE		(1 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
+#define MEDIA_ENT_T_DEVNODE		MEDIA_ENT_T_V4L2_BASE
+#define MEDIA_ENT_T_V4L2_SUBDEV		MEDIA_ENT_T_V4L2_SUBDEV_BASE
+
+#define MEDIA_ENT_T_DEVNODE_V4L		MEDIA_ENT_T_V4L2_VIDEO
+
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
-#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
-#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
-#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
-#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
+#define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
 
-/* Legacy symbol. Use it to avoid userspace compilation breakages */
-#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
-
-#define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV + 1)
-#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV + 2)
-#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV + 3)
-/* A converter of analogue video to its digital representation. */
-#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
-
-#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
+/* Entity types */
 
 #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
 

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

Instead of abusing MEDIA_ENT_T_V4L2_SUBDEV, initialize
new subdev entities as MEDIA_ENT_T_UNKNOWN.

Signed-off-by: Mauro Carvalho Chehab <redacted>

Acked-by: Hans Verkuil <redacted>

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

This change should be moved to patch 38.

  * Drivers should change it to something useful
  */
 #define MEDIA_ENT_T_UNKNOWN	0x00000000

Regards,

	Hans

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

Put the legacy MEDIA_ENT_* macros under a #ifndef __KERNEL__,
in order to be sure that none of those old symbols are used
inside the Kernel.

Signed-off-by: Mauro Carvalho Chehab <redacted>

Acked-by: Hans Verkuil <redacted>

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

s/todo/to do/

+ *	 The API definition should be freed from fields that are bound to
+ *	 some specific data structure.
+ *
+ * FIXME: Currently, I opted to name the new types as "media_v2", as this
+ *	  won't cause any conflict with the Kernelspace namespace, nor with
+ *	  the previous kAPI media_*_desc namespace. This can be changed
+ *	  latter, before the adding this API upstream.

s/latter/later/ :-)

I think this comment belongs to the commit log and not in this header.

+ */
+
+
+#define MEDIA_NEW_LNK_FL_ENABLED		MEDIA_LNK_FL_ENABLED
+#define MEDIA_NEW_LNK_FL_IMMUTABLE		MEDIA_LNK_FL_IMMUTABLE
+#define MEDIA_NEW_LNK_FL_DYNAMIC		MEDIA_NEW_FL_DYNAMIC
+#define MEDIA_NEW_LNK_FL_INTERFACE_LINK		(1 << 3)

Shouldn't this be MEDIA_V2_ instead of MEDIA_NEW_?

Do we need the INTERFACE_LINK flag? You can deduce it by checking the
ID type.

I don't have a clear preference one way or another, just wondering about the
reason for adding it.

+
+struct media_v2_entity {
+	__u32 id;
+	char name[64];		/* FIXME: move to a property? (RFC says so) */
+	__u16 reserved[14];
+};
+
+/* Should match the specific fields at media_intf_devnode */
+struct media_v2_intf_devnode {
+	__u32 major;
+	__u32 minor;
+};
+
+struct media_v2_interface {
+	__u32 id;
+	__u32 intf_type;
+	__u32 flags;
+	__u32 reserved[9];
+
+	union {
+		struct media_v2_intf_devnode devnode;
+		__u32 raw[16];
+	};
+};
+
+struct media_v2_pad {
+	__u32 id;
+	__u32 entity_id;
+	__u32 flags;
+	__u16 reserved[9];
+};
+
+struct media_v2_link {
+    __u32 id;
+    __u32 source_id;
+    __u32 sink_id;

Like in media_link I would use a union here as well to be able to refer to
source/sink_id and entity/interface_id.

+    __u32 flags;
+    __u32 reserved[5];
+};
+
+struct media_v2_topology {
+	__u32 topology_version;
+
+	__u32 num_entities;
+	struct media_v2_entity *entities;
+
+	__u32 num_interfaces;
+	struct media_v2_interface *interfaces;
+
+	__u32 num_pads;
+	struct media_v2_pad *pads;
+
+	__u32 num_links;
+	struct media_v2_link *links;
+
+	__u32 reserved[64];

As mentioned before: use this instead to prevent horrible 32/64 bit arch
compat code:

	struct {
		__u32 reserved_num;
		void *reserved_ptr;
	} reserved_types[16];
	__u32 reserved[8];

Sizes for these arrays are TBD.

+};
+
+/* ioctls */
 
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)
 #define MEDIA_IOC_SETUP_LINK		_IOWR('|', 0x03, struct media_link_desc)
+#define MEDIA_IOC_G_TOPOLOGY		_IOWR('|', 0x04, struct media_v2_topology)
 
 #endif /* __LINUX_MEDIA_H */

Regards,

	Hans

Em Mon, 31 Aug 2015 13:17:08 +0200
Hans Verkuil [off-list ref] escreveu:

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

quoted

Now that interfaces got created, we need to fix the entity
namespace.

So, let's create a consistent new namespace and add backward
compatibility macros to keep the old namespace preserved.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 14d32cdcdd47..88013d1a2c39 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c

@@ -230,17 +230,17 @@ static void dvb_create_media_entity(struct dvb_device *dvbdev,
 
 	switch (type) {
 	case DVB_DEVICE_FRONTEND:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DEMUX:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_CA:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;

@@ -439,7 +439,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
 void dvb_create_media_graph(struct dvb_adapter *adap)
 {
 	struct media_device *mdev = adap->mdev;
-	struct media_entity *entity, *tuner = NULL, *fe = NULL;
+	struct media_entity *entity, *tuner = NULL, *demod = NULL;
 	struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
 	struct media_interface *intf;
 

@@ -451,26 +451,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
 		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
 			tuner = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_FE:
-			fe = entity;
+		case MEDIA_ENT_T_DVB_DEMOD:
+			demod = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
+		case MEDIA_ENT_T_DVB_DEMUX:
 			demux = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
+		case MEDIA_ENT_T_DVB_TSOUT:
 			dvr = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_CA:
+		case MEDIA_ENT_T_DVB_CA:
 			ca = entity;
 			break;
 		}
 	}
 
-	if (tuner && fe)
-		media_create_pad_link(tuner, 0, fe, 0, 0);
+	if (tuner && demod)
+		media_create_pad_link(tuner, 0, demod, 0, 0);
 
-	if (fe && demux)
-		media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
+	if (demod && demux)
+		media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
 
 	if (demux && dvr)
 		media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index aca828709bad..3bbda409353f 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -42,31 +42,71 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
+/*
+ * Base numbers for entity types
+ *
+ * Please notice that the huge gap of 16 bits for each base is overkill!
+ * 8 bits is more than enough to avoid starving entity types for each
+ * subsystem.
+ *
+ * However, It is kept this way just to avoid binary breakages with the
+ * namespace provided on legacy versions of this header.
+ */
+#define MEDIA_ENT_T_DVB_BASE		0x00000001

I would change this to 0x00000000, see follow-up comment later for why.

quoted

+#define MEDIA_ENT_T_V4L2_BASE		0x00010000
+#define MEDIA_ENT_T_V4L2_SUBDEV_BASE	0x00020000
+
+/*
+ * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
+ *	read()/write() data I/O associated with the V4L2 devnodes.
+ */
+#define MEDIA_ENT_T_V4L2_VIDEO		(MEDIA_ENT_T_V4L2_BASE + 1)
+	/*
+	 * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
+	 * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
+	 * to be declared for FB, ALSA and DVB entities.
+	 * As those values were never actually used in practice, we're just
+	 * adding them as backward compatibility macros and keeping the
+	 * numberspace clean here. This way, we avoid breaking compilation,
+	 * in the case of having some userspace application using the old
+	 * symbols.
+	 */
+#define MEDIA_ENT_T_V4L2_VBI		(MEDIA_ENT_T_V4L2_BASE + 5)
+	/* for TX radio, as RX is done via either ALSA or wire */
+#define MEDIA_ENT_T_V4L2_RADIO		(MEDIA_ENT_T_V4L2_BASE + 6)

But TX is also done via either ALSA or wire. This shouldn't be needed.

OK. I'll drop it.

quoted

+#define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 7)

How about MEDIA_ENT_T_DVB_IO_* and MEDIA_ENT_T_V4L2_IO_* to indicate that
this entity deals with data I/O?

Or, perhaps even better, MEDIA_ENT_T_IO_DVB_ and MEDIA_ENT_T_IO_V4L2_.

Works for me.

Entities should do something, and just saying 'V4L2_VIDEO' doesn't really convey
that meaning. It is also very easy to confuse with INTF_T_V4L_* types. BTW, we
should decide whether V4L2 or V4L is used here (interfaces now use V4L, entities
V4L2). Since entities already use V4L2, I think the interface defines should
use V4L2 as well.

Yes, agreed. We actually need to discuss a little more about
namespacing and do some additional renaming stuff.

For example, calling a tuner entity as MEDIA_ENT_T_V4L2_SUBDEV_TUNER
is wrong, because the tuner can be used only at the DVB side.

So, both V4L2 and SUBDEV prefixes there are wrong. Yet, this should be
under the V4L2_SUBDEV range to avoid breaking userspace. 

quoted

+
+/* V4L2 Sub-device entities */
+#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
+#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
+#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
+	/* A converter of analogue video to its digital representation. */
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
+	/* Tuner entity is actually both V4L2 and DVB subdev */
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
+
+/* DVB entities */
+#define MEDIA_ENT_T_DVB_DEMOD		(MEDIA_ENT_T_DVB_BASE)

After changing DVB_BASE to 0, change this to DVB_BASE + 1, and adjust the other DVB
entity types accordingly.

This keeps the base defines consistent (i.e. the lower 16 bits are always 0).

It surprised me when reading this patch, so I'm probably not the only one.

This is another thing for discussion: keeping the MEDIA_ENT_T_foo_BASE
unused opens space for API abuse.

There are several entities at OMAP3 driver, for example, that keeps
entity->type undefined. So, they got whatever is the default at
v4l2-device.c (before this series: MEDIA_ENT_T_V4L2_SUBDEV, after
that: MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN):

$ media-ctl --print-t|grep -B1 Unknown
- entity 1: OMAP3 ISP CCP2 (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
--
- entity 3: OMAP3 ISP CSI2a (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
--
- entity 5: OMAP3 ISP CCDC (3 pads, 8 links)
            type V4L2 subdev subtype Unknown flags 0
--
- entity 7: OMAP3 ISP preview (2 pads, 4 links)
            type V4L2 subdev subtype Unknown flags 0
--
- entity 10: OMAP3 ISP resizer (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
--
- entity 13: OMAP3 ISP AEWB (1 pad, 1 link)
             type V4L2 subdev subtype Unknown flags 0
--
- entity 14: OMAP3 ISP AF (1 pad, 1 link)
             type V4L2 subdev subtype Unknown flags 0
--
- entity 15: OMAP3 ISP histogram (1 pad, 1 link)
             type V4L2 subdev subtype Unknown flags 0

I guess all the above entities are processing units, so they
should have, instead, some type like:
	MEDIA_ENT_T_V4L2_SUBDEV_PROCESSING

Or, even some of the above would actually deserves to have an specific
type, like:
	MEDIA_ENT_T_V4L2_SUBDEV_HISTOGRAM
	MEDIA_ENT_T_V4L2_SUBDEV_RESIZER
	...

Let's try to find some time to discuss the entities namespace on IRC during
this week.

Regards,
Mauro

Em Mon, 31 Aug 2015 14:00:42 +0200
Hans Verkuil [off-list ref] escreveu:

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:

quoted

Add a new ioctl that will report the entire topology on
one go.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 756e1960fd7f..358a0c6b1f86 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h

@@ -181,6 +181,8 @@ struct media_interface {
  */
 struct media_intf_devnode {
 	struct media_interface		intf;
+
+	/* Should match the fields at media_v2_intf_devnode */
 	u32				major;
 	u32				minor;
 };

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4186891e5e81..fa0b68e670b0 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h

@@ -251,11 +251,94 @@ struct media_links_enum {
 #define MEDIA_INTF_T_ALSA_RAWMIDI       (MEDIA_INTF_T_ALSA_BASE + 4)
 #define MEDIA_INTF_T_ALSA_HWDEP         (MEDIA_INTF_T_ALSA_BASE + 5)
 
-/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
+/*
+ * MC next gen API definitions
+ *
+ * NOTE: The declarations below are close to the MC RFC for the Media
+ *	 Controller, the next generation. Yet, there are a few adjustments
+ *	 to do, as we want to be able to have a functional API before
+ *	 the MC properties change. Those will be properly marked below.
+ *	 Please also notice that I removed "num_pads", "num_links",
+ *	 from the proposal, as a proper userspace application will likely
+ *	 use lists for pads/links, just as we intend todo in Kernelspace.

s/todo/to do/

quoted

+ *	 The API definition should be freed from fields that are bound to
+ *	 some specific data structure.
+ *
+ * FIXME: Currently, I opted to name the new types as "media_v2", as this
+ *	  won't cause any conflict with the Kernelspace namespace, nor with
+ *	  the previous kAPI media_*_desc namespace. This can be changed
+ *	  latter, before the adding this API upstream.

s/latter/later/ :-)

I think this comment belongs to the commit log and not in this header.

True, but I opted to keep it here for now to produce some discussions ;)

I'm actually in doubt if we should rename the flags as proposed below,
and use the newer flags only at G_TOPOLOGY or if we should keep the same
namespace for them and accept newer flags with legacy ioctls.

quoted

+ */
+
+
+#define MEDIA_NEW_LNK_FL_ENABLED		MEDIA_LNK_FL_ENABLED
+#define MEDIA_NEW_LNK_FL_IMMUTABLE		MEDIA_LNK_FL_IMMUTABLE
+#define MEDIA_NEW_LNK_FL_DYNAMIC		MEDIA_NEW_FL_DYNAMIC
+#define MEDIA_NEW_LNK_FL_INTERFACE_LINK		(1 << 3)

Shouldn't this be MEDIA_V2_ instead of MEDIA_NEW_?

Do we need the INTERFACE_LINK flag? You can deduce it by checking the
ID type.

Yes, this can be deduced from the type of the objects inside the link.

I guess I added it because of some comment from your media.h RFC
proposal.

Right now, I'm using it at the application to better represent the graph
elements:

	http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen&id=fdc16ece9732c94cfa76eee86978158c5976c00a#n438 
	http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen&id=fdc16ece9732c94cfa76eee86978158c5976c00a#n444

But it could, instead, be doing something like:

	if media_type(link->gobj1.id == MEDIA_GRAPH_PAD)
		link_is_pad = true;
	else
		link_is_pad = false;


Btw, I'm using the same type for both data and interface links, as
I don't see any reason why to differentiate internally: they all share
the same linked list at mdev and the same object ID range.

I don't have a clear preference one way or another, just wondering about the
reason for adding it.

quoted

+
+struct media_v2_entity {
+	__u32 id;
+	char name[64];		/* FIXME: move to a property? (RFC says so) */
+	__u16 reserved[14];
+};
+
+/* Should match the specific fields at media_intf_devnode */
+struct media_v2_intf_devnode {
+	__u32 major;
+	__u32 minor;
+};
+
+struct media_v2_interface {
+	__u32 id;
+	__u32 intf_type;
+	__u32 flags;
+	__u32 reserved[9];
+
+	union {
+		struct media_v2_intf_devnode devnode;
+		__u32 raw[16];
+	};
+};
+
+struct media_v2_pad {
+	__u32 id;
+	__u32 entity_id;
+	__u32 flags;
+	__u16 reserved[9];
+};
+
+struct media_v2_link {
+    __u32 id;
+    __u32 source_id;
+    __u32 sink_id;

Like in media_link I would use a union here as well to be able to refer to
source/sink_id and entity/interface_id.

That would be overkill, and won't help.

Unions make the code harder to read and kernel-doc-nano doesn't like unions
very much.

Ok, there are some cases where it helps, but there's no good reason
for doing it here.

If you don't like the name, let's just rename it to something else.

quoted

+    __u32 flags;
+    __u32 reserved[5];
+};
+
+struct media_v2_topology {
+	__u32 topology_version;
+
+	__u32 num_entities;
+	struct media_v2_entity *entities;
+
+	__u32 num_interfaces;
+	struct media_v2_interface *interfaces;
+
+	__u32 num_pads;
+	struct media_v2_pad *pads;
+
+	__u32 num_links;
+	struct media_v2_link *links;
+
+	__u32 reserved[64];

As mentioned before: use this instead to prevent horrible 32/64 bit arch
compat code:

	struct {
		__u32 reserved_num;
		void *reserved_ptr;
	} reserved_types[16];
	__u32 reserved[8];

Sizes for these arrays are TBD.

OK. Sorry, I forgot to do this change.

quoted

+};
+
+/* ioctls */
 
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)
 #define MEDIA_IOC_SETUP_LINK		_IOWR('|', 0x03, struct media_link_desc)
+#define MEDIA_IOC_G_TOPOLOGY		_IOWR('|', 0x04, struct media_v2_topology)
 
 #endif /* __LINUX_MEDIA_H */

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Aug 30, 2015 at 5:06 AM, Mauro Carvalho Chehab
[off-list ref] wrote:

Declare the interface types that will be used by the new
G_TOPOLOGY ioctl that will be defined latter on.

For now, we need those types, as they'll be used on the
internal structs associated with the new media_interface
graph object defined on the next patch.

Signed-off-by: Mauro Carvalho Chehab <redacted>
Acked-by: Hans Verkuil <redacted>

Reviewed-by: Javier Martinez Canillas <redacted>

Best regards,
Javier