[PATCH v8 00/55] MC next generation patches

[PATCH v8 00/55] MC next generation patches

[Mauro Carvalho Chehab]

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

[PATCH v8 13/55] [media] uapi/media.h: Declare interface types for V4L2 and DVB

[Mauro Carvalho Chehab]

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 <mchehab@osg.samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

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

Re: [PATCH v8 13/55] [media] uapi/media.h: Declare interface types for V4L2 and DVB

[Javier Martinez Canillas]

On Sun, Aug 30, 2015 at 5:06 AM, Mauro Carvalho Chehab
<mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> 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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>

Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Best regards,
Javier

[PATCH v8 15/55] [media] uapi/media.h: Declare interface types for ALSA

[Mauro Carvalho Chehab]

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

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

Re: [PATCH v8 15/55] [media] uapi/media.h: Declare interface types for ALSA

[Mauro Carvalho Chehab]

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

Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index dc679dfe8ade..27fce6224972 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

Re: [PATCH v8 15/55] [media] uapi/media.h: Declare interface types for ALSA

[Javier Martinez Canillas]

On Sun, Sep 6, 2015 at 2:02 PM, Mauro Carvalho Chehab
<mchehab@osg.samsung.com> wrote:
> Declare the interface types to be used on alsa for the new
> G_TOPOLOGY ioctl.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
Javier

Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Mauro Carvalho Chehab]

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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index ada0738d26f2..dadcf1655070 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..f8725b881a1d 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -42,31 +42,69 @@ 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		0x00000000
+#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)
+#define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 6)
+
+/* 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 + 1)
+#define MEDIA_ENT_T_DVB_DEMUX		(MEDIA_ENT_T_DVB_BASE + 2)
+#define MEDIA_ENT_T_DVB_TSOUT		(MEDIA_ENT_T_DVB_BASE + 3)
+#define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 4)
+#define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 5)
+
+/* 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

Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Hans Verkuil]

On 09/06/2015 02:02 PM, Mauro Carvalho Chehab wrote:
> 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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index ada0738d26f2..dadcf1655070 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..f8725b881a1d 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,31 +42,69 @@ 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		0x00000000
> +#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)
> +#define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 6)
> +
> +/* 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 + 1)
> +#define MEDIA_ENT_T_DVB_DEMUX		(MEDIA_ENT_T_DVB_BASE + 2)
> +#define MEDIA_ENT_T_DVB_TSOUT		(MEDIA_ENT_T_DVB_BASE + 3)
> +#define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 4)
> +#define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 5)
> +
> +/* 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)
>  
> 

Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Mauro Carvalho Chehab]

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

Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 220864319d21..7320cdc45833 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 3d6210095336..f90147cb9b57 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -42,8 +42,6 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
-/* Used values for media_entity_desc::type */
-
 /*
  * Initial value to be used when a new entity is created
  * Drivers should change it to something useful
-- 
2.4.3

Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Hans Verkuil]

On 09/06/2015 02:02 PM, Mauro Carvalho Chehab wrote:
> Don't use anymore the type/subtype entity data/macros
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 220864319d21..7320cdc45833 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 3d6210095336..f90147cb9b57 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,8 +42,6 @@ struct media_device_info {
>  
>  #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
>  
> -/* Used values for media_entity_desc::type */
> -
>  /*
>   * Initial value to be used when a new entity is created
>   * Drivers should change it to something useful
> 

Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:02:59 Mauro Carvalho Chehab wrote:
> Don't use anymore the type/subtype entity data/macros
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 220864319d21..7320cdc45833 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 3d6210095336..f90147cb9b57 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,8 +42,6 @@ struct media_device_info {
> 
>  #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
> 
> -/* Used values for media_entity_desc::type */
> -
>  /*
>   * Initial value to be used when a new entity is created
>   * Drivers should change it to something useful

-- 
Regards,

Laurent Pinchart

Re: [PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel

[Mauro Carvalho Chehab]

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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index f90147cb9b57..a1bd7afba110 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -105,6 +105,7 @@ struct media_device_info {
 #define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 4)
 #define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 5)
 
+#ifndef __KERNEL__
 /* Legacy symbols used to avoid userspace compilation breakages */
 #define MEDIA_ENT_TYPE_SHIFT		16
 #define MEDIA_ENT_TYPE_MASK		0x00ff0000
@@ -118,6 +119,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

Re: [PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:03:00 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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index f90147cb9b57..a1bd7afba110 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -105,6 +105,7 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 4)
>  #define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 5)
> 
> +#ifndef __KERNEL__
>  /* Legacy symbols used to avoid userspace compilation breakages */
>  #define MEDIA_ENT_TYPE_SHIFT		16
>  #define MEDIA_ENT_TYPE_MASK		0x00ff0000
> @@ -118,6 +119,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 */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

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

Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 7320cdc45833..2d5ad40254b7 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 a1bd7afba110..b17f6763aff4 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -206,6 +206,10 @@ struct media_pad_desc {
 #define MEDIA_LNK_FL_IMMUTABLE		(1 << 1)
 #define MEDIA_LNK_FL_DYNAMIC		(1 << 2)
 
+#define MEDIA_LNK_FL_LINK_TYPE		(0xf << 28)
+#  define MEDIA_LNK_FL_DATA_LINK	(0 << 28)
+#  define MEDIA_LNK_FL_INTERFACE_LINK	(1 << 28)
+
 struct media_link_desc {
 	struct media_pad_desc source;
 	struct media_pad_desc sink;
@@ -249,11 +253,93 @@ 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 to do 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
+ *	  later, before the adding this API upstream.
+ */
+
+
+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;
+
+	struct {
+		__u32 reserved_num;
+		void *reserved_ptr;
+	} reserved_types[16];
+	__u32 reserved[8];
+};
+
+/* 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

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Hans Verkuil]

On 09/06/2015 02:03 PM, Mauro Carvalho Chehab wrote:
> Add a new ioctl that will report the entire topology on
> one go.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

I do have one suggestion: media_v2_interface, media_v2_pad and media_v2_link all
have a 'flags' field, but it is not clear from the code in the header which flag
#defines are used for which struct. Perhaps a few comments would help with that.

> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index a1bd7afba110..b17f6763aff4 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h

<snip>

> +
> +struct media_v2_pad {
> +	__u32 id;
> +	__u32 entity_id;
> +	__u32 flags;
> +	__u16 reserved[9];

Strange odd number here for no clear reason. Perhaps use 8 or 10 instead?

Anyway, you have my Ack.

Regards,

	Hans

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:03:04 Mauro Carvalho Chehab wrote:
> Add a new ioctl that will report the entire topology on
> one go.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 7320cdc45833..2d5ad40254b7 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 a1bd7afba110..b17f6763aff4 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -206,6 +206,10 @@ struct media_pad_desc {
>  #define MEDIA_LNK_FL_IMMUTABLE		(1 << 1)
>  #define MEDIA_LNK_FL_DYNAMIC		(1 << 2)
> 
> +#define MEDIA_LNK_FL_LINK_TYPE		(0xf << 28)
> +#  define MEDIA_LNK_FL_DATA_LINK	(0 << 28)
> +#  define MEDIA_LNK_FL_INTERFACE_LINK	(1 << 28)
> +
>  struct media_link_desc {
>  	struct media_pad_desc source;
>  	struct media_pad_desc sink;
> @@ -249,11 +253,93 @@ 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 to do 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
> + *	  later, before the adding this API upstream.

Yes, that's a good idea. Or at least we need to remove this comment if we 
decide to keep the v2 names :-)

> + */
> +
> +
> +struct media_v2_entity {
> +	__u32 id;
> +	char name[64];		/* FIXME: move to a property? (RFC says so) */

I agree with Sakari that we can keep the name here even if we also expose it 
as a property. However, there's one issue we need to address : we need to 
clearly define what the name field should contain and how it should be 
constructed, otherwise we'll end up with the exact same mess as today, and I 
don't want that. We can discuss it in this mail thread or as replies to a 
future documentation patch.

> +	__u16 reserved[14];

Sakari and Hans have already commented on using __u32 instead of __u16 for 
reserved fields, as well as on the number of reserved fields. I agree with 
them but have nothing to add.

> +};
> +
> +/* 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;

The kernel seems to be moving to using __u64 instead of pointers in userspace-
facing structures to avoid compat32 code.

> +
> +	__u32 num_interfaces;
> +	struct media_v2_interface *interfaces;
> +
> +	__u32 num_pads;
> +	struct media_v2_pad *pads;
> +
> +	__u32 num_links;
> +	struct media_v2_link *links;
> +
> +	struct {
> +		__u32 reserved_num;
> +		void *reserved_ptr;
> +	} reserved_types[16];
> +	__u32 reserved[8];

I'd just create __u32 reserved fields without any reserved_types, we can 
always use the reserved fields to add new types later.

> +};
> +
> +/* 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,

Laurent Pinchart

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

Em Sun, 06 Dec 2015 02:47:39 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Sunday 06 September 2015 09:03:04 Mauro Carvalho Chehab wrote:
> > Add a new ioctl that will report the entire topology on
> > one go.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 7320cdc45833..2d5ad40254b7 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 a1bd7afba110..b17f6763aff4 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -206,6 +206,10 @@ struct media_pad_desc {
> >  #define MEDIA_LNK_FL_IMMUTABLE		(1 << 1)
> >  #define MEDIA_LNK_FL_DYNAMIC		(1 << 2)
> > 
> > +#define MEDIA_LNK_FL_LINK_TYPE		(0xf << 28)
> > +#  define MEDIA_LNK_FL_DATA_LINK	(0 << 28)
> > +#  define MEDIA_LNK_FL_INTERFACE_LINK	(1 << 28)
> > +
> >  struct media_link_desc {
> >  	struct media_pad_desc source;
> >  	struct media_pad_desc sink;
> > @@ -249,11 +253,93 @@ 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 to do 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
> > + *	  later, before the adding this API upstream.
> 
> Yes, that's a good idea. Or at least we need to remove this comment if we 
> decide to keep the v2 names :-)

True ;)

> 
> > + */
> > +
> > +
> > +struct media_v2_entity {
> > +	__u32 id;
> > +	char name[64];		/* FIXME: move to a property? (RFC says so) */
> 
> I agree with Sakari that we can keep the name here even if we also expose it 
> as a property. However, there's one issue we need to address : we need to 
> clearly define what the name field should contain and how it should be 
> constructed, otherwise we'll end up with the exact same mess as today, and I 
> don't want that. We can discuss it in this mail thread or as replies to a 
> future documentation patch.


Well, let's discuss it then. What's your proposal?

I guess there are actually some different goals that we want to archive
with "name":

1) an ideally short name used when displaying an entity on some graph. 
It would be good if such "short name" would be unique, but it won't
hurt much if such name is not unique;

2) an unique ID identifier that will likely take the position of an
entity inside the machine bus, like PCI ID, USB ID, I2C bus + I2C addr...

I guess such UID would be better addressed via properties, as it
would likely be constructed in userspace via a different set of
properties, depending on the device and/or bus type.

> 
> > +	__u16 reserved[14];
> 
> Sakari and Hans have already commented on using __u32 instead of __u16 for 
> reserved fields, as well as on the number of reserved fields. I agree with 
> them but have nothing to add.

Ok. I'll change this on a later patch.

> 
> > +};
> > +
> > +/* 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;
> 
> The kernel seems to be moving to using __u64 instead of pointers in userspace-
> facing structures to avoid compat32 code.

We had such discussion at the MC summit. I don't object to change to
__u64, but we need to reach a consensus ;)

> 
> > +
> > +	__u32 num_interfaces;
> > +	struct media_v2_interface *interfaces;
> > +
> > +	__u32 num_pads;
> > +	struct media_v2_pad *pads;
> > +
> > +	__u32 num_links;
> > +	struct media_v2_link *links;
> > +
> > +	struct {
> > +		__u32 reserved_num;
> > +		void *reserved_ptr;
> > +	} reserved_types[16];
> > +	__u32 reserved[8];
> 
> I'd just create __u32 reserved fields without any reserved_types, we can 
> always use the reserved fields to add new types later.

It used to be __u32... Hans requested to change it to an struct.

Btw, I also prefer to use __u32 here ;)

> 
> > +};
> > +
> > +/* 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 */
> 

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Arnd Bergmann]

On Tuesday 08 December 2015 17:23:40 Mauro Carvalho Chehab wrote:
> > > +
> > > +struct media_v2_topology {
> > > +   __u32 topology_version;
> > > +
> > > +   __u32 num_entities;
> > > +   struct media_v2_entity *entities;
> > 
> > The kernel seems to be moving to using __u64 instead of pointers in userspace-
> > facing structures to avoid compat32 code.
> 
> We had such discussion at the MC summit. I don't object to change to
> __u64, but we need to reach a consensus 
> 

Just saw the email fly by. Using a __u64 to pass a pointer is generally
the preferred method for most subsystems these days.

However, that means you probably want to extract the pointer from
that using something like

static inline void __user *get_upointer(u64 arg)
{
	return (void __user *)(uintptr_t)arg;
}

This is the only way that I know that works on both 32-bit and 64-bit
architectures as well as the oddball s390 compat mode (which you don't
care about because there are no media devices on s390).

	Arnd

[PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Mauro Carvalho Chehab]

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 <mchehab@osg.samsung.com>

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

Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> 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 <mchehab@osg.samsung.com>
> 
> 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.

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

Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 13:17:08 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > 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 <mchehab@osg.samsung.com>
> > 
> > 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.
> 
> > +#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.

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

> 
> > +
> > +/* 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

[PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Mauro Carvalho Chehab]

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

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

Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Hans Verkuil]

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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

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

Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Mauro Carvalho Chehab]

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

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

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 f8725b881a1d..3d6210095336 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 to be used when a new entity is created
+ * Drivers should change it to something useful
+ */
+#define MEDIA_ENT_T_UNKNOWN	0x00000000
+
 /*
  * Base numbers for entity types
  *
@@ -75,6 +83,15 @@ struct media_device_info {
 #define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 6)
 
 /* 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

Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

In addition to my reply to Sakari's e-mail, please see below for a few small 
comments.

On Sunday 06 September 2015 09:02:58 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 <mchehab@osg.samsung.com>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> 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 f8725b881a1d..3d6210095336 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 */
> +

You remove this a couple of patches later, is it worth adding it in the first 
place ?

> +/*
> + * Initial value to be used when a new entity is created
> + * Drivers should change it to something useful

As you warn when the value isn't change I'd say "Drivers must change...".

> + */
> +#define MEDIA_ENT_T_UNKNOWN	0x00000000
> +
>  /*
>   * Base numbers for entity types
>   *
> @@ -75,6 +83,15 @@ struct media_device_info {
>  #define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 6)
> 
>  /* 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.
> +	 */

Leading tabs look weird here.

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

-- 
Regards,

Laurent Pinchart

Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Mauro Carvalho Chehab]

Em Sun, 06 Dec 2015 03:37:59 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> In addition to my reply to Sakari's e-mail, please see below for a few small 
> comments.
> 
> On Sunday 06 September 2015 09:02:58 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 <mchehab@osg.samsung.com>
> > Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > 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 f8725b881a1d..3d6210095336 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 */
> > +
> 
> You remove this a couple of patches later, is it worth adding it in the first 
> place ?

If you had come with that earlier, I would happily have it removed ;)
Right now, I really want to avoid changes on the patches. Handling a
83 patch series, with two ~20 patch series depending on it (Shuah's
ALSA patches and Sakari's internal entity numbering series) is not
fun.

So, as this is already removed on some other patch, I'll let this
hunk as-is.

> 
> > +/*
> > + * Initial value to be used when a new entity is created
> > + * Drivers should change it to something useful
> 
> As you warn when the value isn't change I'd say "Drivers must change...".

I'll fix this on a later patch.

> > + */
> > +#define MEDIA_ENT_T_UNKNOWN	0x00000000
> > +
> >  /*
> >   * Base numbers for entity types
> >   *
> > @@ -75,6 +83,15 @@ struct media_device_info {
> >  #define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 6)
> > 
> >  /* 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.
> > +	 */
> 
> Leading tabs look weird here.

Ok, I'll remove the ident here. This hunk had to be conflict solved
anyway, as we merged the MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN addition
earlier. I'll do this specific change here, and hope to not cause
context conflicts later - it will likely cause - as we renamed from
ENT_T_foo to ENT_F_foo :( Let's hope git is smart enough here...

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

[PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Mauro Carvalho Chehab]

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

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

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

Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Don't use anymore the type/subtype entity data/macros
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> 
> 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

This change should be moved to patch 38.

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

Regards,

	Hans

[PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel

[Mauro Carvalho Chehab]

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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

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

Re: [PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel

[Hans Verkuil]

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 <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

> 
> 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 */
>  
> 

[PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

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

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

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

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Add a new ioctl that will report the entire topology on
> one go.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> 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/

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

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 14:00:42 +0200
Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Add a new ioctl that will report the entire topology on
> > one go.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > 
> > 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/
> 
> > + *	 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.

> 
> > + */
> > +
> > +
> > +#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.
> 
> > +
> > +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.

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

> 
> > +};
> > +
> > +/* 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

Re: [PATCH v8 00/55] MC next generation patches

[Mauro Carvalho Chehab]

Em Sun, 30 Aug 2015 00:06:11 -0300
Mauro Carvalho Chehab <mchehab@osg.samsung.com> 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