[PATCHv4 00/25] dvb core: add basic support for the media controller

[PATCHv4 00/25] dvb core: add basic support for the media controller

[Mauro Carvalho Chehab]

This patch series adds basic support for the media controller at the
DVB core: it creates one media entity per DVB devnode, if the media
device is passed as an argument to the DVB structures.

The cx231xx driver was modified to pass such argument for DVB NET,
DVB frontend and DVB demux.

-

version 4:

- Addressed the issues pointed via e-mail
- Added a separate Kconfig option to enable media controller DVB
  experimental support
- Fixed some CodingStyle issues
- Added documentation for the API changes at the DocBook

version 3:
- Added the second series of patches ("add link graph to cx231xx 
  using the media controller")
- tuner-core and cx25840: add proper error handling as suggested by
  Sakari Ailus and pointed by Joe Perches;
- dvb core: move the media_dev struct to be inside the DVB adapter. That
  allowed to simplify the changes for the dvbdev clients;
- Add logic to setup the pipelines when analog or digital TV stream starts.
- Renamed some patches to better describe its contents.

version 2:
- Now the PADs are created for all nodes
- Instead of using entity->flags for subtypes, create separate
  MEDIA_ENT_T_DEVNODE_DVB_foo for each DVB devtype
- The API change patch was split from the DVB core changes

Mauro Carvalho Chehab (24):
  [media] media: Fix DVB devnode representation at media controller
  [media] Docbook: Fix documentation for media controller devnodes
  [media] media: add new types for DVB devnodes
  [media] DocBook: Document the DVB API devnodes at the media controller
  [media] media: add a subdev type for tuner
  [media] DocBook: Add tuner subdev at documentation
  [media] dvbdev: add support for media controller
  [media] cx231xx: add media controller support
  [media] dvb_frontend: add media controller support for DVB frontend
  [media] dmxdev: add support for demux/dvr nodes at media controller
  [media] dvb_ca_en50221: add support for CA node at the media
    controller
  [media] dvb_net: add support for DVB net node at the media controller
  [media] dvbdev: add pad for the DVB devnodes
  [media] tuner-core: properly initialize media controller subdev
  [media] cx25840: fill the media controller entity
  [media] cx231xx: initialize video/vbi pads
  [media] cx231xx: create media links for analog mode
  [media] dvbdev: represent frontend with two pads
  [media] dvbdev: add a function to create DVB media graph
  [media] cx231xx: create DVB graph
  [media] dvbdev: enable DVB-specific links
  [media] dvb-frontend: enable tuner link when the FE thread starts
  [media] cx231xx: enable tuner->decoder link at videobuf start
  [media] dvb_frontend: start media pipeline while thread is running

Zhangfei Gao (1):
  [media] ir-hix5hd2: remove writel/readl_relaxed define

 .../DocBook/media/v4l/media-ioc-enum-entities.xml  | 102 ++++-----------
 Documentation/DocBook/media/v4l/v4l2.xml           |   9 ++
 drivers/media/Kconfig                              |  10 +-
 drivers/media/dvb-core/dmxdev.c                    |  11 +-
 drivers/media/dvb-core/dvb_ca_en50221.c            |   6 +-
 drivers/media/dvb-core/dvb_frontend.c              | 121 ++++++++++++++++-
 drivers/media/dvb-core/dvb_net.c                   |   6 +-
 drivers/media/dvb-core/dvbdev.c                    | 143 ++++++++++++++++++++-
 drivers/media/dvb-core/dvbdev.h                    |  15 +++
 drivers/media/i2c/cx25840/cx25840-core.c           |  18 +++
 drivers/media/i2c/cx25840/cx25840-core.h           |   3 +
 drivers/media/rc/ir-hix5hd2.c                      |   8 --
 drivers/media/usb/cx231xx/cx231xx-cards.c          |  98 +++++++++++++-
 drivers/media/usb/cx231xx/cx231xx-dvb.c            |   5 +
 drivers/media/usb/cx231xx/cx231xx-video.c          |  84 +++++++++++-
 drivers/media/usb/cx231xx/cx231xx.h                |   5 +
 drivers/media/v4l2-core/tuner-core.c               |  20 +++
 drivers/media/v4l2-core/v4l2-dev.c                 |   4 +-
 drivers/media/v4l2-core/v4l2-device.c              |   4 +-
 include/media/media-entity.h                       |  12 +-
 include/uapi/linux/media.h                         |  26 +++-
 21 files changed, 592 insertions(+), 118 deletions(-)

-- 
2.1.0

[PATCHv4 01/25] [media] ir-hix5hd2: remove writel/readl_relaxed define

[Mauro Carvalho Chehab]

From: Zhangfei Gao <zhangfei.gao@linaro.org>

Commit 9439eb3ab9d1ec ("asm-generic: io: implement relaxed
accessor macros as conditional wrappers") has added
{read,write}{b,w,l,q}_relaxed to include/asm-generic/io.h

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/rc/ir-hix5hd2.c b/drivers/media/rc/ir-hix5hd2.c
index b0df62961c14..58ec5986274e 100644
--- a/drivers/media/rc/ir-hix5hd2.c
+++ b/drivers/media/rc/ir-hix5hd2.c
@@ -16,14 +16,6 @@
 #include <linux/regmap.h>
 #include <media/rc-core.h>
 
-/* Allow the driver to compile on all architectures */
-#ifndef writel_relaxed
-# define writel_relaxed writel
-#endif
-#ifndef readl_relaxed
-# define readl_relaxed readl
-#endif
-
 #define IR_ENABLE		0x00
 #define IR_CONFIG		0x04
 #define CNT_LEADS		0x08
-- 
2.1.0

[PATCHv4 02/25] [media] media: Fix DVB devnode representation at media controller

[Mauro Carvalho Chehab]

The previous provision for DVB media controller support were to
define an ID (likely meaning the adapter number) for the DVB
devnodes.

This is just plain wrong. Just like V4L, DVB devices (and any other
device node)) are uniquely identified via a (major, minor) tuple.

This is enough to uniquely identify a devnode, no matter what
API it implements.

So, before we go too far, let's mark the old v4l, fb, dvb and alsa
"devnode" info as deprecated, and just call it as "dev".

We can latter add fields specific to each API if needed.

As we don't want to break compilation on already existing apps,
let's just keep the old definitions as-is, adding a note that
those are deprecated at media-entity.h.

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

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 86bb93fd7db8..d89d5cb465d9 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -943,8 +943,8 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 	    vdev->vfl_type != VFL_TYPE_SUBDEV) {
 		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
 		vdev->entity.name = vdev->name;
-		vdev->entity.info.v4l.major = VIDEO_MAJOR;
-		vdev->entity.info.v4l.minor = vdev->minor;
+		vdev->entity.info.dev.major = VIDEO_MAJOR;
+		vdev->entity.info.dev.minor = vdev->minor;
 		ret = media_device_register_entity(vdev->v4l2_dev->mdev,
 			&vdev->entity);
 		if (ret < 0)
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 015f92aab44a..204cc67c84e8 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -248,8 +248,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
 			goto clean_up;
 		}
 #if defined(CONFIG_MEDIA_CONTROLLER)
-		sd->entity.info.v4l.major = VIDEO_MAJOR;
-		sd->entity.info.v4l.minor = vdev->minor;
+		sd->entity.info.dev.major = VIDEO_MAJOR;
+		sd->entity.info.dev.minor = vdev->minor;
 #endif
 		sd->devnode = vdev;
 	}
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index e00459185d20..d6d74bcfe183 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -87,17 +87,7 @@ struct media_entity {
 		struct {
 			u32 major;
 			u32 minor;
-		} v4l;
-		struct {
-			u32 major;
-			u32 minor;
-		} fb;
-		struct {
-			u32 card;
-			u32 device;
-			u32 subdevice;
-		} alsa;
-		int dvb;
+		} dev;
 
 		/* Sub-device specifications */
 		/* Nothing needed yet */
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index d847c760e8f0..418f4fec391a 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -78,6 +78,20 @@ struct media_entity_desc {
 		struct {
 			__u32 major;
 			__u32 minor;
+		} dev;
+
+#if 1
+		/*
+		 * DEPRECATED: previous node specifications. Kept just to
+		 * avoid breaking compilation, but media_entity_desc.dev
+		 * should be used instead. In particular, alsa and dvb
+		 * fields below are wrong: for all devnodes, there should
+		 * be just major/minor inside the struct, as this is enough
+		 * to represent any devnode, no matter what type.
+		 */
+		struct {
+			__u32 major;
+			__u32 minor;
 		} v4l;
 		struct {
 			__u32 major;
@@ -89,6 +103,7 @@ struct media_entity_desc {
 			__u32 subdevice;
 		} alsa;
 		int dvb;
+#endif
 
 		/* Sub-device specifications */
 		/* Nothing needed yet */
-- 
2.1.0

[PATCHv4 03/25] [media] Docbook: Fix documentation for media controller devnodes

[Mauro Carvalho Chehab]

The media-ctl userspace application assumes that all device nodes
are uniquelly defined via major,minor, just like v4l and fb.

That's ok for those types of devices, but, as we're adding support
for DVB at the API, what's written there at the DocBook is wrong.

So, fix it.

While here, fix the size of the reserved space inside the union,
with is 184, and not 180.

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

diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
index 116c301656e0..19ab836b2651 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
@@ -143,86 +143,14 @@
 	  <row>
 	    <entry></entry>
 	    <entry>struct</entry>
-	    <entry><structfield>v4l</structfield></entry>
+	    <entry><structfield>dev</structfield></entry>
 	    <entry></entry>
-	    <entry>Valid for V4L sub-devices and nodes only.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>major</structfield></entry>
-	    <entry>V4L device node major number. For V4L sub-devices with no
-	    device node, set by the driver to 0.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>minor</structfield></entry>
-	    <entry>V4L device node minor number. For V4L sub-devices with no
-	    device node, set by the driver to 0.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry>struct</entry>
-	    <entry><structfield>fb</structfield></entry>
-	    <entry></entry>
-	    <entry>Valid for frame buffer nodes only.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>major</structfield></entry>
-	    <entry>Frame buffer device node major number.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>minor</structfield></entry>
-	    <entry>Frame buffer device node minor number.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry>struct</entry>
-	    <entry><structfield>alsa</structfield></entry>
-	    <entry></entry>
-	    <entry>Valid for ALSA devices only.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>card</structfield></entry>
-	    <entry>ALSA card number</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>device</structfield></entry>
-	    <entry>ALSA device number</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry>__u32</entry>
-	    <entry><structfield>subdevice</structfield></entry>
-	    <entry>ALSA sub-device number</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry>int</entry>
-	    <entry><structfield>dvb</structfield></entry>
-	    <entry></entry>
-	    <entry>DVB card number</entry>
+	    <entry>Valid for (sub-)devices that create devnodes.</entry>
 	  </row>
 	  <row>
 	    <entry></entry>
 	    <entry>__u8</entry>
-	    <entry><structfield>raw</structfield>[180]</entry>
+	    <entry><structfield>raw</structfield>[184]</entry>
 	    <entry></entry>
 	    <entry></entry>
 	  </row>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index ac0f8d9d2a49..3a16ac1f67d0 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -136,6 +136,7 @@ Remote Controller chapter.</contrib>
       <year>2012</year>
       <year>2013</year>
       <year>2014</year>
+      <year>2015</year>
       <holder>Bill Dirks, Michael H. Schimek, Hans Verkuil, Martin
 Rubli, Andy Walls, Muralidharan Karicheri, Mauro Carvalho Chehab,
 	Pawel Osciak</holder>
@@ -152,6 +153,13 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->
 
       <revision>
+	<revnumber>3.21</revnumber>
+	<date>2015-02-13</date>
+	<authorinitials>mcc</authorinitials>
+	<revremark>Fix documentation for media controller device nodes.
+	</revremark>
+      </revision>
+      <revision>
 	<revnumber>3.19</revnumber>
 	<date>2014-12-05</date>
 	<authorinitials>hv</authorinitials>
-- 
2.1.0

[PATCHv4 04/25] [media] media: add new types for DVB devnodes

[Mauro Carvalho Chehab]

Most of the DVB subdevs have already their own devnode.

Add support for them at the media controller API.

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

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 418f4fec391a..4c8f26243252 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -50,7 +50,14 @@ struct media_device_info {
 #define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
 #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)
+#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)
+
+/* 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)
-- 
2.1.0

[PATCHv4 05/25] [media] DocBook: Document the DVB API devnodes at the media controller

[Mauro Carvalho Chehab]

The DVB API is actually several different APIs bundled together, each
using its own device node.

Fix the media controller DVB API to actually reflect what's there at the
DVB subsystem.

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

diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
index 19ab836b2651..6f1b1cf172b7 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
@@ -181,8 +181,24 @@
 	    <entry>ALSA card</entry>
 	  </row>
 	  <row>
-	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB</constant></entry>
-	    <entry>DVB card</entry>
+	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_FE</constant></entry>
+	    <entry>DVB frontend devnode</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_DEMUX</constant></entry>
+	    <entry>DVB demux devnode</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_DVR</constant></entry>
+	    <entry>DVB DVR devnode</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_CA</constant></entry>
+	    <entry>DVB CAM devnode</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_NET</constant></entry>
+	    <entry>DVB network devnode</entry>
 	  </row>
 	  <row>
 	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV</constant></entry>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 3a16ac1f67d0..408e1068ffe6 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -156,7 +156,7 @@ applications. -->
 	<revnumber>3.21</revnumber>
 	<date>2015-02-13</date>
 	<authorinitials>mcc</authorinitials>
-	<revremark>Fix documentation for media controller device nodes.
+	<revremark>Fix documentation for media controller device nodes and add support for DVB device nodes.
 	</revremark>
       </revision>
       <revision>
-- 
2.1.0

[PATCHv4 06/25] [media] media: add a subdev type for tuner

[Mauro Carvalho Chehab]

Add MEDIA_ENT_T_V4L2_SUBDEV_TUNER to represent the V4L2
(and dvb) tuner subdevices.

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

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4c8f26243252..52cc2a6b19b7 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -66,6 +66,8 @@ struct media_device_info {
 /* 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)
+
 #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
 
 struct media_entity_desc {
-- 
2.1.0

[PATCHv4 07/25] [media] DocBook: Add tuner subdev at documentation

[Mauro Carvalho Chehab]

Now that we've added MEDIA_ENT_T_V4L2_SUBDEV_TUNER at the API,
document it.

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

diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
index 6f1b1cf172b7..cbf307f21a63 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
@@ -226,6 +226,10 @@
 	    it in some digital video standard, with appropriate embedded timing
 	    signals.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_TUNER</constant></entry>
+	    <entry>TV and/or radio tuner</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 408e1068ffe6..e98caa1c39bd 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -157,6 +157,7 @@ applications. -->
 	<date>2015-02-13</date>
 	<authorinitials>mcc</authorinitials>
 	<revremark>Fix documentation for media controller device nodes and add support for DVB device nodes.
+Add support for Tuner sub-device.
 	</revremark>
       </revision>
       <revision>
-- 
2.1.0

[PATCHv4 08/25] [media] dvbdev: add support for media controller

[Mauro Carvalho Chehab]

Provide a way to register media controller device nodes
at the DVB core.

Please notice that the dvbdev callers also require changes
for the devices to be registered via the media controller.

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

diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index 49cd30870e0d..3ef0f90b128f 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -87,13 +87,21 @@ config MEDIA_RC_SUPPORT
 
 config MEDIA_CONTROLLER
 	bool "Media Controller API"
-	depends on MEDIA_CAMERA_SUPPORT
+	depends on MEDIA_CAMERA_SUPPORT || MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT
 	---help---
 	  Enable the media controller API used to query media devices internal
 	  topology and configure it dynamically.
 
 	  This API is mostly used by camera interfaces in embedded platforms.
 
+config MEDIA_CONTROLLER_DVB
+	bool "Enable Media controller for DVB"
+	depends on MEDIA_CONTROLLER
+	---help---
+	  Enable the media controller API support for DVB.
+
+	  This is currently experimental.
+
 #
 # Video4Linux support
 #	Only enables if one of the V4L2 types (ATV, webcam, radio) is selected
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 983db75de350..f98fd3b29afe 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -180,6 +180,59 @@ skip:
 	return -ENFILE;
 }
 
+static void dvb_register_media_device(struct dvb_device *dvbdev,
+				      int type, int minor)
+{
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	int ret;
+
+	if (!dvbdev->adapter->mdev)
+		return;
+
+	dvbdev->entity = kzalloc(sizeof(*dvbdev->entity), GFP_KERNEL);
+	if (!dvbdev->entity)
+		return;
+
+	dvbdev->entity->info.dev.major = DVB_MAJOR;
+	dvbdev->entity->info.dev.minor = minor;
+	dvbdev->entity->name = dvbdev->name;
+	switch (type) {
+	case DVB_DEVICE_FRONTEND:
+		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
+		break;
+	case DVB_DEVICE_DEMUX:
+		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
+		break;
+	case DVB_DEVICE_DVR:
+		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
+		break;
+	case DVB_DEVICE_CA:
+		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
+		break;
+	case DVB_DEVICE_NET:
+		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
+		break;
+	default:
+		kfree(dvbdev->entity);
+		dvbdev->entity = NULL;
+		return;
+	}
+
+	ret = media_device_register_entity(dvbdev->adapter->mdev,
+					   dvbdev->entity);
+	if (ret < 0) {
+		printk(KERN_ERR
+			"%s: media_device_register_entity failed for %s\n",
+			__func__, dvbdev->entity->name);
+		kfree(dvbdev->entity);
+		dvbdev->entity = NULL;
+		return;
+	}
+
+	printk(KERN_DEBUG "%s: media device '%s' registered.\n",
+		__func__, dvbdev->entity->name);
+#endif
+}
 
 int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 			const struct dvb_device *template, void *priv, int type)
@@ -258,10 +311,11 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 		       __func__, adap->num, dnames[type], id, PTR_ERR(clsdev));
 		return PTR_ERR(clsdev);
 	}
-
 	dprintk(KERN_DEBUG "DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
 		adap->num, dnames[type], id, minor, minor);
 
+	dvb_register_media_device(dvbdev, type, minor);
+
 	return 0;
 }
 EXPORT_SYMBOL(dvb_register_device);
@@ -278,6 +332,13 @@ void dvb_unregister_device(struct dvb_device *dvbdev)
 
 	device_destroy(dvb_class, MKDEV(DVB_MAJOR, dvbdev->minor));
 
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	if (dvbdev->entity) {
+		media_device_unregister_entity(dvbdev->entity);
+		kfree(dvbdev->entity);
+	}
+#endif
+
 	list_del (&dvbdev->list_head);
 	kfree (dvbdev->fops);
 	kfree (dvbdev);
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index f96b28e7fc95..485d8e660aea 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -27,6 +27,7 @@
 #include <linux/poll.h>
 #include <linux/fs.h>
 #include <linux/list.h>
+#include <media/media-device.h>
 
 #define DVB_MAJOR 212
 
@@ -71,6 +72,10 @@ struct dvb_adapter {
 	int mfe_shared;			/* indicates mutually exclusive frontends */
 	struct dvb_device *mfe_dvbdev;	/* frontend device in use */
 	struct mutex mfe_lock;		/* access lock for thread creation */
+
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	struct media_device *mdev;
+#endif
 };
 
 
@@ -92,6 +97,14 @@ struct dvb_device {
 	/* don't really need those !? -- FIXME: use video_usercopy  */
 	int (*kernel_ioctl)(struct file *file, unsigned int cmd, void *arg);
 
+	/* Needed for media controller register/unregister */
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	const char *name;
+
+	/* Filled inside dvbdev.c */
+	struct media_entity *entity;
+#endif
+
 	void *priv;
 };
 
-- 
2.1.0

[PATCHv4 09/25] [media] cx231xx: add media controller support

[Mauro Carvalho Chehab]

Let's add media controller support for this driver and register it
for both V4L and DVB.

The media controller on this driver is not mandatory, as it can fully
work without it. So, if the media controller register fails, just print
an error message, but proceed with device registering.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index da03733690bd..d357e8c0c485 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -912,9 +912,6 @@ static inline void cx231xx_set_model(struct cx231xx *dev)
  */
 void cx231xx_pre_card_setup(struct cx231xx *dev)
 {
-
-	cx231xx_set_model(dev);
-
 	dev_info(dev->dev, "Identified as %s (card=%d)\n",
 		dev->board.name, dev->model);
 
@@ -1092,6 +1089,17 @@ void cx231xx_config_i2c(struct cx231xx *dev)
 	call_all(dev, video, s_stream, 1);
 }
 
+static void cx231xx_unregister_media_device(struct cx231xx *dev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	if (dev->media_dev) {
+		media_device_unregister(dev->media_dev);
+		kfree(dev->media_dev);
+		dev->media_dev = NULL;
+	}
+#endif
+}
+
 /*
  * cx231xx_realease_resources()
  * unregisters the v4l2,i2c and usb devices
@@ -1099,6 +1107,8 @@ void cx231xx_config_i2c(struct cx231xx *dev)
 */
 void cx231xx_release_resources(struct cx231xx *dev)
 {
+	cx231xx_unregister_media_device(dev);
+
 	cx231xx_release_analog_resources(dev);
 
 	cx231xx_remove_from_devlist(dev);
@@ -1117,6 +1127,38 @@ void cx231xx_release_resources(struct cx231xx *dev)
 	clear_bit(dev->devno, &cx231xx_devused);
 }
 
+static void cx231xx_media_device_register(struct cx231xx *dev,
+					  struct usb_device *udev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	struct media_device *mdev;
+	int ret;
+
+	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+	if (!mdev)
+		return;
+
+	mdev->dev = dev->dev;
+	strlcpy(mdev->model, dev->board.name, sizeof(mdev->model));
+	if (udev->serial)
+		strlcpy(mdev->serial, udev->serial, sizeof(mdev->serial));
+	strcpy(mdev->bus_info, udev->devpath);
+	mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
+	mdev->driver_version = LINUX_VERSION_CODE;
+
+	ret = media_device_register(mdev);
+	if (ret) {
+		dev_err(dev->dev,
+			"Couldn't create a media device. Error: %d\n",
+			ret);
+		kfree(mdev);
+		return;
+	}
+
+	dev->media_dev = mdev;
+#endif
+}
+
 /*
  * cx231xx_init_dev()
  * allocates and inits the device structs, registers i2c bus and v4l device
@@ -1225,10 +1267,8 @@ static int cx231xx_init_dev(struct cx231xx *dev, struct usb_device *udev,
 	}
 
 	retval = cx231xx_register_analog_devices(dev);
-	if (retval) {
-		cx231xx_release_analog_resources(dev);
+	if (retval)
 		goto err_analog;
-	}
 
 	cx231xx_ir_init(dev);
 
@@ -1236,6 +1276,8 @@ static int cx231xx_init_dev(struct cx231xx *dev, struct usb_device *udev,
 
 	return 0;
 err_analog:
+	cx231xx_unregister_media_device(dev);
+	cx231xx_release_analog_resources(dev);
 	cx231xx_remove_from_devlist(dev);
 err_dev_init:
 	cx231xx_dev_uninit(dev);
@@ -1438,6 +1480,8 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	dev->video_mode.alt = -1;
 	dev->dev = d;
 
+	cx231xx_set_model(dev);
+
 	dev->interface_count++;
 	/* reset gpio dir and value */
 	dev->gpio_dir = 0;
@@ -1502,7 +1546,11 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	/* save our data pointer in this interface device */
 	usb_set_intfdata(interface, dev);
 
+	/* Register the media controller */
+	cx231xx_media_device_register(dev, udev);
+
 	/* Create v4l2 device */
+	dev->v4l2_dev.mdev = dev->media_dev;
 	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
 	if (retval) {
 		dev_err(d, "v4l2_device_register failed\n");
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index dd600b994e69..bb7e766cd30c 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -455,6 +455,7 @@ static int register_dvb(struct cx231xx_dvb *dvb,
 
 	mutex_init(&dvb->lock);
 
+
 	/* register adapter */
 	result = dvb_register_adapter(&dvb->adapter, dev->name, module, device,
 				      adapter_nr);
@@ -464,6 +465,9 @@ static int register_dvb(struct cx231xx_dvb *dvb,
 		       dev->name, result);
 		goto fail_adapter;
 	}
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+	dvb->adapter.mdev = dev->media_dev;
+#endif
 
 	/* Ensure all frontends negotiate bus access */
 	dvb->frontend->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl;
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 6d6f3ee812f6..af9d6c4041dc 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -658,6 +658,10 @@ struct cx231xx {
 	struct video_device *vbi_dev;
 	struct video_device *radio_dev;
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_device *media_dev;
+#endif
+
 	unsigned char eedata[256];
 
 	struct cx231xx_video_mode video_mode;
-- 
2.1.0

[PATCHv4 10/25] [media] dvb_frontend: add media controller support for DVB frontend

[Mauro Carvalho Chehab]

Now that the dvb core is capable of registering devices via the
media controller, add support for the DVB frontend devices.

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

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 2cf30576bf39..2564422278df 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2612,11 +2612,14 @@ int dvb_register_frontend(struct dvb_adapter* dvb,
 			  struct dvb_frontend* fe)
 {
 	struct dvb_frontend_private *fepriv;
-	static const struct dvb_device dvbdev_template = {
+	const struct dvb_device dvbdev_template = {
 		.users = ~0,
 		.writers = 1,
 		.readers = (~0)-1,
 		.fops = &dvb_frontend_fops,
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+		.name = fe->ops.info.name,
+#endif
 		.kernel_ioctl = dvb_frontend_ioctl
 	};
 
-- 
2.1.0

[PATCHv4 11/25] [media] dmxdev: add support for demux/dvr nodes at media controller

[Mauro Carvalho Chehab]

Make the dvb core demux support aware of the media controller and
register the corresponding devices.

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

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index abff803ad69a..2835924955a4 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -1136,10 +1136,13 @@ static const struct file_operations dvb_demux_fops = {
 	.llseek = default_llseek,
 };
 
-static struct dvb_device dvbdev_demux = {
+static const struct dvb_device dvbdev_demux = {
 	.priv = NULL,
 	.users = 1,
 	.writers = 1,
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	.name = "demux",
+#endif
 	.fops = &dvb_demux_fops
 };
 
@@ -1209,13 +1212,15 @@ static const struct file_operations dvb_dvr_fops = {
 	.llseek = default_llseek,
 };
 
-static struct dvb_device dvbdev_dvr = {
+static const struct dvb_device dvbdev_dvr = {
 	.priv = NULL,
 	.readers = 1,
 	.users = 1,
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	.name = "dvr",
+#endif
 	.fops = &dvb_dvr_fops
 };
-
 int dvb_dmxdev_init(struct dmxdev *dmxdev, struct dvb_adapter *dvb_adapter)
 {
 	int i;
-- 
2.1.0

[PATCHv4 12/25] [media] dvb_ca_en50221: add support for CA node at the media controller

[Mauro Carvalho Chehab]

Make the dvb core CA support aware of the media controller and
register the corresponding devices.

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

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 0aac3096728e..2bf28eb97a64 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1638,15 +1638,17 @@ static const struct file_operations dvb_ca_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct dvb_device dvbdev_ca = {
+static const struct dvb_device dvbdev_ca = {
 	.priv = NULL,
 	.users = 1,
 	.readers = 1,
 	.writers = 1,
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	.name = "ca_en50221",
+#endif
 	.fops = &dvb_ca_fops,
 };
 
-
 /* ******************************************************************************** */
 /* Initialisation/shutdown functions */
 
-- 
2.1.0

Re: [PATCHv4 12/25] [media] dvb_ca_en50221: add support for CA node at the media controller

[Hans Verkuil]

On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> Make the dvb core CA support aware of the media controller and
> register the corresponding devices.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
> index 0aac3096728e..2bf28eb97a64 100644
> --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> @@ -1638,15 +1638,17 @@ static const struct file_operations dvb_ca_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> -static struct dvb_device dvbdev_ca = {
> +static const struct dvb_device dvbdev_ca = {
>  	.priv = NULL,
>  	.users = 1,
>  	.readers = 1,
>  	.writers = 1,
> +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> +	.name = "ca_en50221",

I'd use 'dvb-ca-en50221': the dvb prefix makes it clear that this is a 
dvb core device, and personally I prefer '-' over '_'.

Regards,

	Hans

> +#endif
>  	.fops = &dvb_ca_fops,
>  };
>  
> -
>  /* ******************************************************************************** */
>  /* Initialisation/shutdown functions */
>  
> 

Re: [PATCHv4 12/25] [media] dvb_ca_en50221: add support for CA node at the media controller

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 10:04:50 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> > Make the dvb core CA support aware of the media controller and
> > register the corresponding devices.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
> > index 0aac3096728e..2bf28eb97a64 100644
> > --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> > +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> > @@ -1638,15 +1638,17 @@ static const struct file_operations dvb_ca_fops = {
> >  	.llseek = noop_llseek,
> >  };
> >  
> > -static struct dvb_device dvbdev_ca = {
> > +static const struct dvb_device dvbdev_ca = {
> >  	.priv = NULL,
> >  	.users = 1,
> >  	.readers = 1,
> >  	.writers = 1,
> > +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> > +	.name = "ca_en50221",
> 
> I'd use 'dvb-ca-en50221': the dvb prefix makes it clear that this is a 
> dvb core device, and personally I prefer '-' over '_'.

Ok, will do, in order to standardize. Yet, at dvb, the core uses dvb_foo
for the several c files.

Regards,
Mauro

[PATCHv4 13/25] [media] dvb_net: add support for DVB net node at the media controller

[Mauro Carvalho Chehab]

Make the dvb core network support aware of the media controller and
register the corresponding devices.

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

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index 686d3277dad1..40990058b4bc 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -1462,14 +1462,16 @@ static const struct file_operations dvb_net_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct dvb_device dvbdev_net = {
+static const struct dvb_device dvbdev_net = {
 	.priv = NULL,
 	.users = 1,
 	.writers = 1,
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	.name = "dvb net",
+#endif
 	.fops = &dvb_net_fops,
 };
 
-
 void dvb_net_release (struct dvb_net *dvbnet)
 {
 	int i;
-- 
2.1.0

Re: [PATCHv4 13/25] [media] dvb_net: add support for DVB net node at the media controller

[Hans Verkuil]

On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> Make the dvb core network support aware of the media controller and
> register the corresponding devices.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
> index 686d3277dad1..40990058b4bc 100644
> --- a/drivers/media/dvb-core/dvb_net.c
> +++ b/drivers/media/dvb-core/dvb_net.c
> @@ -1462,14 +1462,16 @@ static const struct file_operations dvb_net_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> -static struct dvb_device dvbdev_net = {
> +static const struct dvb_device dvbdev_net = {
>  	.priv = NULL,
>  	.users = 1,
>  	.writers = 1,
> +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> +	.name = "dvb net",

I would suggest 'dvb-net' rather than 'dvb net' with a space. That's a personal
preference, though.

Regards,

	Hans

> +#endif
>  	.fops = &dvb_net_fops,
>  };
>  
> -
>  void dvb_net_release (struct dvb_net *dvbnet)
>  {
>  	int i;
> 

Re: [PATCHv4 13/25] [media] dvb_net: add support for DVB net node at the media controller

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 10:03:51 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> > Make the dvb core network support aware of the media controller and
> > register the corresponding devices.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Thanks for reviewing this patch series!

> > 
> > diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
> > index 686d3277dad1..40990058b4bc 100644
> > --- a/drivers/media/dvb-core/dvb_net.c
> > +++ b/drivers/media/dvb-core/dvb_net.c
> > @@ -1462,14 +1462,16 @@ static const struct file_operations dvb_net_fops = {
> >  	.llseek = noop_llseek,
> >  };
> >  
> > -static struct dvb_device dvbdev_net = {
> > +static const struct dvb_device dvbdev_net = {
> >  	.priv = NULL,
> >  	.users = 1,
> >  	.writers = 1,
> > +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> > +	.name = "dvb net",
> 
> I would suggest 'dvb-net' rather than 'dvb net' with a space. That's a personal
> preference, though.

Works for me. I'll write a patch changing the names to dvb-foo for the
DVB nodes.

Regards,
Mauro

[PATCHv4 14/25] [media] dvbdev: add pad for the DVB devnodes

[Mauro Carvalho Chehab]

We want to represent the links between the several DVB devnodes,
so let's create PADs for them.

The DVB net devnode is a different matter, as it is not related
to the media stream, but with network. So, at least for now, let's
not add any pad for it.

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 f98fd3b29afe..79c96edf71ef 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -184,7 +184,7 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 				      int type, int minor)
 {
 #if defined(CONFIG_MEDIA_CONTROLLER_DVB)
-	int ret;
+	int ret = 0, npads;
 
 	if (!dvbdev->adapter->mdev)
 		return;
@@ -196,18 +196,46 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 	dvbdev->entity->info.dev.major = DVB_MAJOR;
 	dvbdev->entity->info.dev.minor = minor;
 	dvbdev->entity->name = dvbdev->name;
+
+	switch (type) {
+	case DVB_DEVICE_CA:
+	case DVB_DEVICE_DEMUX:
+		npads = 2;
+		break;
+	case DVB_DEVICE_NET:
+		npads = 0;
+		break;
+	default:
+		npads = 1;
+	}
+
+	if (npads) {
+		dvbdev->pads = kcalloc(npads, sizeof(*dvbdev->pads),
+				       GFP_KERNEL);
+		if (!dvbdev->pads) {
+			kfree(dvbdev->entity);
+			return;
+		}
+	}
+
 	switch (type) {
 	case DVB_DEVICE_FRONTEND:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DEMUX:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SINK;
 		break;
 	case DVB_DEVICE_DVR:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		break;
 	case DVB_DEVICE_CA:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SINK;
 		break;
 	case DVB_DEVICE_NET:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
@@ -218,12 +246,16 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 		return;
 	}
 
-	ret = media_device_register_entity(dvbdev->adapter->mdev,
-					   dvbdev->entity);
+	if (npads)
+		ret = media_entity_init(dvbdev->entity, npads, dvbdev->pads, 0);
+	if (!ret)
+		ret = media_device_register_entity(dvbdev->adapter->mdev,
+						   dvbdev->entity);
 	if (ret < 0) {
 		printk(KERN_ERR
 			"%s: media_device_register_entity failed for %s\n",
 			__func__, dvbdev->entity->name);
+		kfree(dvbdev->pads);
 		kfree(dvbdev->entity);
 		dvbdev->entity = NULL;
 		return;
@@ -336,6 +368,7 @@ void dvb_unregister_device(struct dvb_device *dvbdev)
 	if (dvbdev->entity) {
 		media_device_unregister_entity(dvbdev->entity);
 		kfree(dvbdev->entity);
+		kfree(dvbdev->pads);
 	}
 #endif
 
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index 485d8e660aea..464067c43a35 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -101,8 +101,9 @@ struct dvb_device {
 #if defined(CONFIG_MEDIA_CONTROLLER_DVB)
 	const char *name;
 
-	/* Filled inside dvbdev.c */
+	/* Allocated and filled inside dvbdev.c */
 	struct media_entity *entity;
+	struct media_pad *pads;
 #endif
 
 	void *priv;
-- 
2.1.0

[PATCHv4 15/25] [media] tuner-core: properly initialize media controller subdev

[Mauro Carvalho Chehab]

Properly initialize tuner core subdev at the media controller.

That requires a new subtype at the media controller API.

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

diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
index 559f8372e2eb..9a83b27a7e8f 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -134,6 +134,9 @@ struct tuner {
 	unsigned int        type; /* chip type id */
 	void                *config;
 	const char          *name;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad	pad;
+#endif
 };
 
 /*
@@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
 		t->name = analog_ops->info.name;
 	}
 
+	t->sd.entity.name = t->name;
+
 	tuner_dbg("type set to %s\n", t->name);
 
 	t->mode_mask = new_mode_mask;
@@ -592,6 +597,9 @@ static int tuner_probe(struct i2c_client *client,
 	struct tuner *t;
 	struct tuner *radio;
 	struct tuner *tv;
+#ifdef CONFIG_MEDIA_CONTROLLER
+	int ret;
+#endif
 
 	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
 	if (NULL == t)
@@ -684,6 +692,18 @@ static int tuner_probe(struct i2c_client *client,
 
 	/* Should be just before return */
 register_client:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	t->pad.flags = MEDIA_PAD_FL_SOURCE;
+	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
+	t->sd.entity.name = t->name;
+
+	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
+	if (ret < 0) {
+		tuner_err("failed to initialize media entity!\n");
+		kfree(t);
+		return -ENODEV;
+	}
+#endif
 	/* Sets a default mode */
 	if (t->mode_mask & T_ANALOG_TV)
 		t->mode = V4L2_TUNER_ANALOG_TV;
-- 
2.1.0

Re: [PATCHv4 15/25] [media] tuner-core: properly initialize media controller subdev

[Hans Verkuil]

On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> Properly initialize tuner core subdev at the media controller.
> 
> That requires a new subtype at the media controller API.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> index 559f8372e2eb..9a83b27a7e8f 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -134,6 +134,9 @@ struct tuner {
>  	unsigned int        type; /* chip type id */
>  	void                *config;
>  	const char          *name;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	struct media_pad	pad;
> +#endif
>  };
>  
>  /*
> @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
>  		t->name = analog_ops->info.name;
>  	}
>  
> +	t->sd.entity.name = t->name;
> +
>  	tuner_dbg("type set to %s\n", t->name);
>  
>  	t->mode_mask = new_mode_mask;
> @@ -592,6 +597,9 @@ static int tuner_probe(struct i2c_client *client,
>  	struct tuner *t;
>  	struct tuner *radio;
>  	struct tuner *tv;
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	int ret;
> +#endif
>  
>  	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
>  	if (NULL == t)
> @@ -684,6 +692,18 @@ static int tuner_probe(struct i2c_client *client,
>  
>  	/* Should be just before return */
>  register_client:
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> +	t->sd.entity.name = t->name;

Will this be a unique name in the case of one board with multiple identical tuners?

I don't know if we have any cards like that (my PVR-500 is really two PCI boards on
one PCB).

Laurent, the name should be unique, right? In any case, the spec needs to be updated
to clearly state whether or not the name should be unique.

Regards,

	Hans

> +
> +	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> +	if (ret < 0) {
> +		tuner_err("failed to initialize media entity!\n");
> +		kfree(t);
> +		return -ENODEV;
> +	}
> +#endif
>  	/* Sets a default mode */
>  	if (t->mode_mask & T_ANALOG_TV)
>  		t->mode = V4L2_TUNER_ANALOG_TV;
> 

Re: [PATCHv4 15/25] [media] tuner-core: properly initialize media controller subdev

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 10:10:08 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> > Properly initialize tuner core subdev at the media controller.
> > 
> > That requires a new subtype at the media controller API.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> > index 559f8372e2eb..9a83b27a7e8f 100644
> > --- a/drivers/media/v4l2-core/tuner-core.c
> > +++ b/drivers/media/v4l2-core/tuner-core.c
> > @@ -134,6 +134,9 @@ struct tuner {
> >  	unsigned int        type; /* chip type id */
> >  	void                *config;
> >  	const char          *name;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +	struct media_pad	pad;
> > +#endif
> >  };
> >  
> >  /*
> > @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
> >  		t->name = analog_ops->info.name;
> >  	}
> >  
> > +	t->sd.entity.name = t->name;
> > +
> >  	tuner_dbg("type set to %s\n", t->name);
> >  
> >  	t->mode_mask = new_mode_mask;
> > @@ -592,6 +597,9 @@ static int tuner_probe(struct i2c_client *client,
> >  	struct tuner *t;
> >  	struct tuner *radio;
> >  	struct tuner *tv;
> > +#ifdef CONFIG_MEDIA_CONTROLLER
> > +	int ret;
> > +#endif
> >  
> >  	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
> >  	if (NULL == t)
> > @@ -684,6 +692,18 @@ static int tuner_probe(struct i2c_client *client,
> >  
> >  	/* Should be just before return */
> >  register_client:
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> > +	t->sd.entity.name = t->name;
> 
> Will this be a unique name in the case of one board with multiple identical tuners?

Good point. Well, it should, as otherwise the logs will be confusing, but
we need to double check it, if we have such case.

> I don't know if we have any cards like that (my PVR-500 is really two PCI boards on
> one PCB).

Except for PVR-500, I can't remember any case where the same tuner is used
more than once.

There is the case of a device with two tuners, one for TV and another one
for FM. Yet, on such case, the name of the FM tuner will be different,
anyway. So, I don't think this is a current issue, but if the name should
be unique, then we need to properly document it.

> Laurent, the name should be unique, right? In any case, the spec needs to be updated
> to clearly state whether or not the name should be unique.

Regards,
Mauro

Re: [PATCHv4 15/25] [media] tuner-core: properly initialize media controller subdev

[Devin Heitmueller]

> Except for PVR-500, I can't remember any case where the same tuner is used
> more than once.
>
> There is the case of a device with two tuners, one for TV and another one
> for FM. Yet, on such case, the name of the FM tuner will be different,
> anyway. So, I don't think this is a current issue, but if the name should
> be unique, then we need to properly document it.

Perhaps I've misunderstood the comment, but HVR-2200/2250 and numerous
dib0700 designs are dual DVB tuners.  Neither are like the PVR-500 in
that they are a single entity with two tuners (as opposed to the
PVR-500 which is two PCI devices which happen to be on the same PCB).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCHv4 15/25] [media] tuner-core: properly initialize media controller subdev

[Hans Verkuil]

On 02/16/2015 03:39 PM, Devin Heitmueller wrote:
>> Except for PVR-500, I can't remember any case where the same tuner is used
>> more than once.
>>
>> There is the case of a device with two tuners, one for TV and another one
>> for FM. Yet, on such case, the name of the FM tuner will be different,
>> anyway. So, I don't think this is a current issue, but if the name should
>> be unique, then we need to properly document it.
> 
> Perhaps I've misunderstood the comment, but HVR-2200/2250 and numerous
> dib0700 designs are dual DVB tuners.  Neither are like the PVR-500 in
> that they are a single entity with two tuners (as opposed to the
> PVR-500 which is two PCI devices which happen to be on the same PCB).

DVB, yes, but not analog (V4L2) tuners. For DVB tuners the frontend name
is used as the entity name, which I assumed is unique. It is, right? If
that's not unique, then the same issue is there as well. I have ordered
a dual DVB-T board, but I won't have that for another two weeks.

Regards,

	Hans

Re: [PATCHv4 15/25] [media] tuner-core: properly initialize media controller subdev

[Devin Heitmueller]

Hi Hans,

On Mon, Feb 16, 2015 at 9:46 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 02/16/2015 03:39 PM, Devin Heitmueller wrote:
>>> Except for PVR-500, I can't remember any case where the same tuner is used
>>> more than once.
>>>
>>> There is the case of a device with two tuners, one for TV and another one
>>> for FM. Yet, on such case, the name of the FM tuner will be different,
>>> anyway. So, I don't think this is a current issue, but if the name should
>>> be unique, then we need to properly document it.
>>
>> Perhaps I've misunderstood the comment, but HVR-2200/2250 and numerous
>> dib0700 designs are dual DVB tuners.  Neither are like the PVR-500 in
>> that they are a single entity with two tuners (as opposed to the
>> PVR-500 which is two PCI devices which happen to be on the same PCB).
>
> DVB, yes, but not analog (V4L2) tuners. For DVB tuners the frontend name
> is used as the entity name, which I assumed is unique. It is, right? If
> that's not unique, then the same issue is there as well. I have ordered
> a dual DVB-T board, but I won't have that for another two weeks.

Sorry, I thought this patch was related to the DVB tuner subdev -
didn't realize it was for tuner-core (which is obviously analog).

The HVR-2200/2250 is a single board with dual hybrid tuners (model
2200 has a DVB-T demod and 2250 has an ATSC demod).  In other words,
it has two tuners, each of which can be independently tuned to either
digital or analog signals.  And unlike the PVR-500, it's a single PCIe
bridge (saa7164).

Regarding your question on DVB tuners, yes - each tuner gets its own
frontendX device node (in reality the frontendX points to the digital
demodulator, but ultimately it passes the tuning request off to the
tuner as well).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

[PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Mauro Carvalho Chehab]

Instead of keeping the media controller entity not initialized,
fill it and create the pads for cx25840.

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

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
index 573e08826b9b..bdb5bb6b58da 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
 	int default_volume;
 	u32 id;
 	u16 device_id;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	int ret;
+#endif
 
 	/* Check if the adapter supports the needed features */
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
@@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
 
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	/* TODO: need to represent analog inputs too */
+	state->pads[0].flags = MEDIA_PAD_FL_SINK;	/* Tuner or input */
+	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;	/* Video */
+	state->pads[2].flags = MEDIA_PAD_FL_SOURCE;	/* VBI */
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
+
+	ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
+				state->pads, 0);
+	if (ret < 0) {
+		v4l_info(client, "failed to initialize media entity!\n");
+		kfree(state);
+		return -ENODEV;
+	}
+#endif
 
 	switch (id) {
 	case CX23885_AV:
diff --git a/drivers/media/i2c/cx25840/cx25840-core.h b/drivers/media/i2c/cx25840/cx25840-core.h
index 37bc04217c44..17b409f55445 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.h
+++ b/drivers/media/i2c/cx25840/cx25840-core.h
@@ -64,6 +64,9 @@ struct cx25840_state {
 	wait_queue_head_t fw_wait;    /* wake up when the fw load is finished */
 	struct work_struct fw_work;   /* work entry for fw load */
 	struct cx25840_ir_state *ir_state;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad	pads[3];
+#endif
 };
 
 static inline struct cx25840_state *to_state(struct v4l2_subdev *sd)
-- 
2.1.0

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Hans Verkuil]

On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> Instead of keeping the media controller entity not initialized,
> fill it and create the pads for cx25840.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> index 573e08826b9b..bdb5bb6b58da 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.c
> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
>  	int default_volume;
>  	u32 id;
>  	u16 device_id;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	int ret;
> +#endif
>  
>  	/* Check if the adapter supports the needed features */
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
>  
>  	sd = &state->sd;
>  	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	/* TODO: need to represent analog inputs too */

Which analog inputs? Do you mean 'audio inputs' instead?

Regards,

	Hans

> +	state->pads[0].flags = MEDIA_PAD_FL_SINK;	/* Tuner or input */
> +	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;	/* Video */
> +	state->pads[2].flags = MEDIA_PAD_FL_SOURCE;	/* VBI */
> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +
> +	ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
> +				state->pads, 0);
> +	if (ret < 0) {
> +		v4l_info(client, "failed to initialize media entity!\n");
> +		kfree(state);
> +		return -ENODEV;
> +	}
> +#endif
>  
>  	switch (id) {
>  	case CX23885_AV:
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.h b/drivers/media/i2c/cx25840/cx25840-core.h
> index 37bc04217c44..17b409f55445 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.h
> +++ b/drivers/media/i2c/cx25840/cx25840-core.h
> @@ -64,6 +64,9 @@ struct cx25840_state {
>  	wait_queue_head_t fw_wait;    /* wake up when the fw load is finished */
>  	struct work_struct fw_work;   /* work entry for fw load */
>  	struct cx25840_ir_state *ir_state;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	struct media_pad	pads[3];
> +#endif
>  };
>  
>  static inline struct cx25840_state *to_state(struct v4l2_subdev *sd)
> 

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 10:11:59 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> > Instead of keeping the media controller entity not initialized,
> > fill it and create the pads for cx25840.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> > index 573e08826b9b..bdb5bb6b58da 100644
> > --- a/drivers/media/i2c/cx25840/cx25840-core.c
> > +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> > @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
> >  	int default_volume;
> >  	u32 id;
> >  	u16 device_id;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +	int ret;
> > +#endif
> >  
> >  	/* Check if the adapter supports the needed features */
> >  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> > @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
> >  
> >  	sd = &state->sd;
> >  	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +	/* TODO: need to represent analog inputs too */
> 
> Which analog inputs? Do you mean 'audio inputs' instead?

I mean analog video inputs like composite, svideo, etc, e. g.
having multiple input pads for the analog demods, like:

                ___________
TUNER --------> |         |
                |         |
SVIDEO .......> | cx25840 |
                |         |
COMPOSITE1 ...> |_________|


(in the above, dashes represent the enabled link, and periods
represent the disabled ones)

In other words, if we want to properly represent the pipeline, 
it should be possible to see via the media controller if the tuner
is being used as an image source, or if the source is something else.

I didn't map those other inputs here yet, due to a few things:
- Not sure if it is really worth
- The extra inputs would require subdevs that won't be controlled
- I was in doubt about the best way for doing that
- That would likely require some extra setup for cx25840 caller
  drivers, in order to represent what of the possible internal
  inputs are actually used on each specific board
- I was too lazy to do it ;)

Actually, I didn't see much benefit on adding such map now, so I
decided to just add a comment inside the source code.

Regards,
Mauro

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Hans Verkuil]

On 02/16/2015 12:11 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 16 Feb 2015 10:11:59 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
>>> Instead of keeping the media controller entity not initialized,
>>> fill it and create the pads for cx25840.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>
>>> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
>>> index 573e08826b9b..bdb5bb6b58da 100644
>>> --- a/drivers/media/i2c/cx25840/cx25840-core.c
>>> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
>>> @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
>>>  	int default_volume;
>>>  	u32 id;
>>>  	u16 device_id;
>>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>>> +	int ret;
>>> +#endif
>>>  
>>>  	/* Check if the adapter supports the needed features */
>>>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>>> @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
>>>  
>>>  	sd = &state->sd;
>>>  	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
>>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>>> +	/* TODO: need to represent analog inputs too */
>>
>> Which analog inputs? Do you mean 'audio inputs' instead?
> 
> I mean analog video inputs like composite, svideo, etc, e. g.
> having multiple input pads for the analog demods, like:
> 
>                 ___________
> TUNER --------> |         |
>                 |         |
> SVIDEO .......> | cx25840 |
>                 |         |
> COMPOSITE1 ...> |_________|
> 
> 
> (in the above, dashes represent the enabled link, and periods
> represent the disabled ones)
> 
> In other words, if we want to properly represent the pipeline, 
> it should be possible to see via the media controller if the tuner
> is being used as an image source, or if the source is something else.
> 
> I didn't map those other inputs here yet, due to a few things:
> - Not sure if it is really worth
> - The extra inputs would require subdevs that won't be controlled
> - I was in doubt about the best way for doing that
> - That would likely require some extra setup for cx25840 caller
>   drivers, in order to represent what of the possible internal
>   inputs are actually used on each specific board
> - I was too lazy to do it ;)
> 
> Actually, I didn't see much benefit on adding such map now, so I
> decided to just add a comment inside the source code.

OK, fair enough, I'd have done the same. But if you could extend the comment
to include what you just said, then that will be helpful in the future.

Thanks!

	Hans

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 12:16:07 +0100
Hans Verkuil <hansverk@cisco.com> escreveu:

> On 02/16/2015 12:11 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 16 Feb 2015 10:11:59 +0100
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > 
> >> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> >>> Instead of keeping the media controller entity not initialized,
> >>> fill it and create the pads for cx25840.
> >>>
> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>
> >>> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> >>> index 573e08826b9b..bdb5bb6b58da 100644
> >>> --- a/drivers/media/i2c/cx25840/cx25840-core.c
> >>> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> >>> @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
> >>>  	int default_volume;
> >>>  	u32 id;
> >>>  	u16 device_id;
> >>> +#if defined(CONFIG_MEDIA_CONTROLLER)
> >>> +	int ret;
> >>> +#endif
> >>>  
> >>>  	/* Check if the adapter supports the needed features */
> >>>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> >>> @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
> >>>  
> >>>  	sd = &state->sd;
> >>>  	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> >>> +#if defined(CONFIG_MEDIA_CONTROLLER)
> >>> +	/* TODO: need to represent analog inputs too */
> >>
> >> Which analog inputs? Do you mean 'audio inputs' instead?
> > 
> > I mean analog video inputs like composite, svideo, etc, e. g.
> > having multiple input pads for the analog demods, like:
> > 
> >                 ___________
> > TUNER --------> |         |
> >                 |         |
> > SVIDEO .......> | cx25840 |
> >                 |         |
> > COMPOSITE1 ...> |_________|
> > 
> > 
> > (in the above, dashes represent the enabled link, and periods
> > represent the disabled ones)
> > 
> > In other words, if we want to properly represent the pipeline, 
> > it should be possible to see via the media controller if the tuner
> > is being used as an image source, or if the source is something else.
> > 
> > I didn't map those other inputs here yet, due to a few things:
> > - Not sure if it is really worth
> > - The extra inputs would require subdevs that won't be controlled
> > - I was in doubt about the best way for doing that
> > - That would likely require some extra setup for cx25840 caller
> >   drivers, in order to represent what of the possible internal
> >   inputs are actually used on each specific board
> > - I was too lazy to do it ;)
> > 
> > Actually, I didn't see much benefit on adding such map now, so I
> > decided to just add a comment inside the source code.
> 
> OK, fair enough, I'd have done the same. But if you could extend the comment
> to include what you just said, then that will be helpful in the future.

Sure. Will do.

Thanks!
Mauro

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.

On Fri, Feb 13, 2015 at 10:57 PM, Mauro Carvalho Chehab
<mchehab@osg.samsung.com> wrote:
> Instead of keeping the media controller entity not initialized,
> fill it and create the pads for cx25840.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> index 573e08826b9b..bdb5bb6b58da 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.c
> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
>         int default_volume;
>         u32 id;
>         u16 device_id;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +       int ret;
> +#endif
>
>         /* Check if the adapter supports the needed features */
>         if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
>
>         sd = &state->sd;
>         v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +       /* TODO: need to represent analog inputs too */
> +       state->pads[0].flags = MEDIA_PAD_FL_SINK;       /* Tuner or input */
> +       state->pads[1].flags = MEDIA_PAD_FL_SOURCE;     /* Video */
> +       state->pads[2].flags = MEDIA_PAD_FL_SOURCE;     /* VBI */
Macros for 0,1,2 would make it more readable.

> +       sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +
> +       ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
> +                               state->pads, 0);
> +       if (ret < 0) {
> +               v4l_info(client, "failed to initialize media entity!\n");
> +               kfree(state);
not needed as state is allocated using devm_kzalloc()

> +               return -ENODEV;
return ret instead ?

> +       }
> +#endif
>
>         switch (id) {
>         case CX23885_AV:
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.h b/drivers/media/i2c/cx25840/cx25840-core.h
> index 37bc04217c44..17b409f55445 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.h
> +++ b/drivers/media/i2c/cx25840/cx25840-core.h
> @@ -64,6 +64,9 @@ struct cx25840_state {
>         wait_queue_head_t fw_wait;    /* wake up when the fw load is finished */
>         struct work_struct fw_work;   /* work entry for fw load */
>         struct cx25840_ir_state *ir_state;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +       struct media_pad        pads[3];
Macro for 3 ?

Cheers,
--Prabhakar Lad

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Mauro Carvalho Chehab]

Em Wed, 18 Feb 2015 22:48:04 +0000
"Lad, Prabhakar" <prabhakar.csengg@gmail.com> escreveu:

> Hi Mauro,
> 
> Thanks for the patch.

Thanks for the review.
> 
> On Fri, Feb 13, 2015 at 10:57 PM, Mauro Carvalho Chehab
> <mchehab@osg.samsung.com> wrote:
> > Instead of keeping the media controller entity not initialized,
> > fill it and create the pads for cx25840.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >
> > diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> > index 573e08826b9b..bdb5bb6b58da 100644
> > --- a/drivers/media/i2c/cx25840/cx25840-core.c
> > +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> > @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
> >         int default_volume;
> >         u32 id;
> >         u16 device_id;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +       int ret;
> > +#endif
> >
> >         /* Check if the adapter supports the needed features */
> >         if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> > @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
> >
> >         sd = &state->sd;
> >         v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +       /* TODO: need to represent analog inputs too */
> > +       state->pads[0].flags = MEDIA_PAD_FL_SINK;       /* Tuner or input */
> > +       state->pads[1].flags = MEDIA_PAD_FL_SOURCE;     /* Video */
> > +       state->pads[2].flags = MEDIA_PAD_FL_SOURCE;     /* VBI */
> Macros for 0,1,2 would make it more readable.

I was in doubt, on weather use a macro or not for it. I ended by
deciding to not use because the code shouldn't assume a particular order
for the pads. Also, I'm not sure if is there a way to "taint" a PAD for
VBI or Video, or if it is worth or not do do it.

So, the comments there are more a reminder than anything else.

> 
> > +       sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> > +
> > +       ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
> > +                               state->pads, 0);
> > +       if (ret < 0) {
> > +               v4l_info(client, "failed to initialize media entity!\n");
> > +               kfree(state);
> not needed as state is allocated using devm_kzalloc()
> 
> > +               return -ENODEV;
> return ret instead ?

Yeah, both comments make sense. I'll write a patch changing it.

> 
> > +       }
> > +#endif
> >
> >         switch (id) {
> >         case CX23885_AV:
> > diff --git a/drivers/media/i2c/cx25840/cx25840-core.h b/drivers/media/i2c/cx25840/cx25840-core.h
> > index 37bc04217c44..17b409f55445 100644
> > --- a/drivers/media/i2c/cx25840/cx25840-core.h
> > +++ b/drivers/media/i2c/cx25840/cx25840-core.h
> > @@ -64,6 +64,9 @@ struct cx25840_state {
> >         wait_queue_head_t fw_wait;    /* wake up when the fw load is finished */
> >         struct work_struct fw_work;   /* work entry for fw load */
> >         struct cx25840_ir_state *ir_state;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +       struct media_pad        pads[3];
> Macro for 3 ?

Not much sense, as this is used only here. Ok, if we're adding an
enum (or defines) for the pads, then it makes sense to have an alias
for the number of pads.

Regards,
Mauro

Re: [PATCHv4 16/25] [media] cx25840: fill the media controller entity

[Mauro Carvalho Chehab]

Em Thu, 19 Feb 2015 17:50:07 -0200
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:

> Em Wed, 18 Feb 2015 22:48:04 +0000
> "Lad, Prabhakar" <prabhakar.csengg@gmail.com> escreveu:
> 
> > Hi Mauro,
> > 
> > Thanks for the patch.
> 
> Thanks for the review.
> > 
> > On Fri, Feb 13, 2015 at 10:57 PM, Mauro Carvalho Chehab
> > <mchehab@osg.samsung.com> wrote:
> > > Instead of keeping the media controller entity not initialized,
> > > fill it and create the pads for cx25840.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > >
> > > diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> > > index 573e08826b9b..bdb5bb6b58da 100644
> > > --- a/drivers/media/i2c/cx25840/cx25840-core.c
> > > +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> > > @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client,
> > >         int default_volume;
> > >         u32 id;
> > >         u16 device_id;
> > > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > > +       int ret;
> > > +#endif
> > >
> > >         /* Check if the adapter supports the needed features */
> > >         if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> > > @@ -5178,6 +5181,21 @@ static int cx25840_probe(struct i2c_client *client,
> > >
> > >         sd = &state->sd;
> > >         v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> > > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > > +       /* TODO: need to represent analog inputs too */
> > > +       state->pads[0].flags = MEDIA_PAD_FL_SINK;       /* Tuner or input */
> > > +       state->pads[1].flags = MEDIA_PAD_FL_SOURCE;     /* Video */
> > > +       state->pads[2].flags = MEDIA_PAD_FL_SOURCE;     /* VBI */
> > Macros for 0,1,2 would make it more readable.
> 
> I was in doubt, on weather use a macro or not for it. I ended by
> deciding to not use because the code shouldn't assume a particular order
> for the pads. Also, I'm not sure if is there a way to "taint" a PAD for
> VBI or Video, or if it is worth or not do do it.
> 
> So, the comments there are more a reminder than anything else.

On a second thought, indeed it seems better to use an enum here. Just
sent the patch.

Regards,
Mauro

[PATCHv4 17/25] [media] cx231xx: initialize video/vbi pads

[Mauro Carvalho Chehab]

Both video and vbi are sink pads. Initialize them as such.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index ecea76fe07f6..f3d1a488dfa7 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -2121,7 +2121,12 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 		dev_err(dev->dev, "cannot allocate video_device.\n");
 		return -ENODEV;
 	}
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	dev->video_pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_init(&dev->vdev->entity, 1, &dev->video_pad, 0);
+	if (ret < 0)
+		dev_err(dev->dev, "failed to initialize video media entity!\n");
+#endif
 	dev->vdev->ctrl_handler = &dev->ctrl_handler;
 	/* register v4l2 video video_device */
 	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
@@ -2147,6 +2152,12 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 		dev_err(dev->dev, "cannot allocate video_device.\n");
 		return -ENODEV;
 	}
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	dev->vbi_pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_init(&dev->vbi_dev->entity, 1, &dev->vbi_pad, 0);
+	if (ret < 0)
+		dev_err(dev->dev, "failed to initialize vbi media entity!\n");
+#endif
 	dev->vbi_dev->ctrl_handler = &dev->ctrl_handler;
 	/* register v4l2 vbi video_device */
 	ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index af9d6c4041dc..e0d3106f6b44 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -660,6 +660,7 @@ struct cx231xx {
 
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	struct media_device *media_dev;
+	struct media_pad video_pad, vbi_pad;
 #endif
 
 	unsigned char eedata[256];
-- 
2.1.0

[PATCHv4 18/25] [media] cx231xx: create media links for analog mode

[Mauro Carvalho Chehab]

Now that we have entities and pads, let's create media links
between them, for analog setup.

We may not have all the links for digital yet, as the dvb extention
may not be loaded yet.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index d357e8c0c485..dfc7010cff7f 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1159,6 +1159,42 @@ static void cx231xx_media_device_register(struct cx231xx *dev,
 #endif
 }
 
+static void cx231xx_create_media_graph(struct cx231xx *dev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	struct media_device *mdev = dev->media_dev;
+	struct media_entity *entity;
+	struct media_entity *tuner = NULL, *decoder = NULL;
+
+	if (!mdev)
+		return;
+
+	media_device_for_each_entity(entity, mdev) {
+		switch (entity->type) {
+		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
+			tuner = entity;
+			break;
+		case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
+			decoder = entity;
+			break;
+		}
+	}
+
+	/* Analog setup, using tuner as a link */
+
+	if (!decoder)
+		return;
+
+	if (tuner)
+		media_entity_create_link(tuner, 0, decoder, 0,
+					 MEDIA_LNK_FL_ENABLED);
+	media_entity_create_link(decoder, 1, &dev->vdev->entity, 0,
+				 MEDIA_LNK_FL_ENABLED);
+	media_entity_create_link(decoder, 2, &dev->vbi_dev->entity, 0,
+				 MEDIA_LNK_FL_ENABLED);
+#endif
+}
+
 /*
  * cx231xx_init_dev()
  * allocates and inits the device structs, registers i2c bus and v4l device
@@ -1616,6 +1652,8 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	/* load other modules required */
 	request_modules(dev);
 
+	cx231xx_create_media_graph(dev);
+
 	return 0;
 err_video_alt:
 	/* cx231xx_uninit_dev: */
-- 
2.1.0

[PATCHv4 19/25] [media] dvbdev: represent frontend with two pads

[Mauro Carvalho Chehab]

While on some devices the tuner is bound inside the frontend,
other devices use a separate subdevice for it.

So, in order to be more generic, better to map it with two
pads.

That will allows to use the media controller to lock the tuner
between the DVB and the V4L2 sub-drivers, on hybrid devices.

While here, change the logic to use pad 0 as sink for devices
with both sink and source pads.

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 79c96edf71ef..c5de02455b17 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -200,6 +200,7 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 	switch (type) {
 	case DVB_DEVICE_CA:
 	case DVB_DEVICE_DEMUX:
+	case DVB_DEVICE_FRONTEND:
 		npads = 2;
 		break;
 	case DVB_DEVICE_NET:
@@ -221,12 +222,13 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 	switch (type) {
 	case DVB_DEVICE_FRONTEND:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
-		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
+		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->pads[0].flags = MEDIA_PAD_FL_SOURCE;
-		dvbdev->pads[1].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DVR:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
@@ -234,8 +236,8 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 		break;
 	case DVB_DEVICE_CA:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
-		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
-		dvbdev->pads[1].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_NET:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
-- 
2.1.0

[PATCHv4 20/25] [media] dvbdev: add a function to create DVB media graph

[Mauro Carvalho Chehab]

We need to create a DVB graph, linking the several DVB devnodes.

Add such function. Please notice that this helper function
doesn't take into account devices with multiple DVB adapters
and frontends.

For devices with multiple adapters, they should either create two
different media controller instances or to improve this function
to take the adapter ID into account.

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 c5de02455b17..a991819ed1e1 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -380,6 +380,51 @@ void dvb_unregister_device(struct dvb_device *dvbdev)
 }
 EXPORT_SYMBOL(dvb_unregister_device);
 
+
+void dvb_create_media_graph(struct media_device *mdev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+	struct media_entity *entity, *tuner = NULL, *fe = NULL;
+	struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
+
+	if (!mdev)
+		return;
+
+	media_device_for_each_entity(entity, mdev) {
+		switch (entity->type) {
+		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
+			tuner = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_FE:
+			fe = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
+			demux = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
+			dvr = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_CA:
+			ca = entity;
+			break;
+		}
+	}
+
+	if (tuner && fe)
+		media_entity_create_link(tuner, 0, fe, 0, 0);
+
+	if (fe && demux)
+		media_entity_create_link(fe, 1, demux, 0, 0);
+
+	if (demux && dvr)
+		media_entity_create_link(demux, 1, dvr, 0, 0);
+
+	if (demux && ca)
+		media_entity_create_link(demux, 1, ca, 0, 0);
+#endif
+}
+EXPORT_SYMBOL_GPL(dvb_create_media_graph);
+
 static int dvbdev_check_free_adapter_num(int num)
 {
 	struct list_head *entry;
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index 464067c43a35..467c1311bd4c 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -122,6 +122,7 @@ extern int dvb_register_device (struct dvb_adapter *adap,
 				int type);
 
 extern void dvb_unregister_device (struct dvb_device *dvbdev);
+void dvb_create_media_graph(struct media_device *mdev);
 
 extern int dvb_generic_open (struct inode *inode, struct file *file);
 extern int dvb_generic_release (struct inode *inode, struct file *file);
-- 
2.1.0

[PATCHv4 21/25] [media] cx231xx: create DVB graph

[Mauro Carvalho Chehab]

cx231xx is simple with regards to DVB: all boards have just one
DVB adapter. So, we can use the default DVB helper function to
create the DVB media graph.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index bb7e766cd30c..e8c054c4ac8c 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -540,6 +540,7 @@ static int register_dvb(struct cx231xx_dvb *dvb,
 
 	/* register network adapter */
 	dvb_net_init(&dvb->adapter, &dvb->net, &dvb->demux.dmx);
+	dvb_create_media_graph(dev->media_dev);
 	return 0;
 
 fail_fe_conn:
-- 
2.1.0

[PATCHv4 22/25] [media] dvbdev: enable DVB-specific links

[Mauro Carvalho Chehab]

For now, let's keep the DVB-specific media controller links enabled
by default. On most devices, this is fixed anyway, so no big issue.

Ok, the demux actually have dynamic links based on the filters, but
we don't represent them yet, as the media controller currently lacks
the capability of dynamically create/delete entities.

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 a991819ed1e1..0af9d0c5f889 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -414,13 +414,13 @@ void dvb_create_media_graph(struct media_device *mdev)
 		media_entity_create_link(tuner, 0, fe, 0, 0);
 
 	if (fe && demux)
-		media_entity_create_link(fe, 1, demux, 0, 0);
+		media_entity_create_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
 
 	if (demux && dvr)
-		media_entity_create_link(demux, 1, dvr, 0, 0);
+		media_entity_create_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
 
 	if (demux && ca)
-		media_entity_create_link(demux, 1, ca, 0, 0);
+		media_entity_create_link(demux, 1, ca, 0, MEDIA_LNK_FL_ENABLED);
 #endif
 }
 EXPORT_SYMBOL_GPL(dvb_create_media_graph);
-- 
2.1.0

[PATCHv4 23/25] [media] dvb-frontend: enable tuner link when the FE thread starts

[Mauro Carvalho Chehab]

If the dvb frontend thread starts, the tuner should be switched
to the frontend. Add a code that ensures that this will happen,
using the media controller.

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

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 2564422278df..50bc6056e914 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -590,12 +590,99 @@ static void dvb_frontend_wakeup(struct dvb_frontend *fe)
 	wake_up_interruptible(&fepriv->wait_queue);
 }
 
+/**
+ * dvb_enable_media_tuner() - tries to enable the DVB tuner
+ *
+ * @fe:		struct dvb_frontend pointer
+ *
+ * This function ensures that just one media tuner is enabled for a given
+ * frontend. It has two different behaviors:
+ * - For trivial devices with just one tuner:
+ *   it just enables the existing tuner->fe link
+ * - For devices with more than one tuner:
+ *   It is up to the driver to implement the logic that will enable one tuner
+ *   and disable the other ones. However, if more than one tuner is enabled for
+ *   the same frontend, it will print an error message and return -EINVAL.
+ *
+ * At return, it will return the error code returned by media_entity_setup_link,
+ * or 0 if everything is OK, if no tuner is linked to the frontend or if the
+ * mdev is NULL.
+ */
+static int dvb_enable_media_tuner(struct dvb_frontend *fe)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+	struct dvb_frontend_private *fepriv = fe->frontend_priv;
+	struct dvb_adapter *adapter = fe->dvb;
+	struct media_device *mdev = adapter->mdev;
+	struct media_entity  *entity, *source;
+	struct media_link *link, *found_link = NULL;
+	int i, ret, n_links = 0, active_links = 0;
+
+	if (!mdev)
+		return 0;
+
+	entity = fepriv->dvbdev->entity;
+	for (i = 0; i < entity->num_links; i++) {
+		link = &entity->links[i];
+		if (link->sink->entity == entity) {
+			found_link = link;
+			n_links++;
+			if (link->flags & MEDIA_LNK_FL_ENABLED)
+				active_links++;
+		}
+	}
+
+	if (!n_links || active_links == 1 || !found_link)
+		return 0;
+
+	/*
+	 * If a frontend has more than one tuner linked, it is up to the driver
+	 * to select with one will be the active one, as the frontend core can't
+	 * guess. If the driver doesn't do that, it is a bug.
+	 */
+	if (n_links > 1 && active_links != 1) {
+		dev_err(fe->dvb->device,
+			"WARNING: there are %d active links among %d tuners. This is a driver's bug!\n",
+			active_links, n_links);
+		return -EINVAL;
+	}
+
+	source = found_link->source->entity;
+	for (i = 0; i < source->num_links; i++) {
+		struct media_entity *sink;
+		int flags = 0;
+
+		link = &source->links[i];
+		sink = link->sink->entity;
+
+		if (sink == entity)
+			flags = MEDIA_LNK_FL_ENABLED;
+
+		ret = media_entity_setup_link(link, flags);
+		if (ret) {
+			dev_err(fe->dvb->device,
+				"Couldn't change link %s->%s to %s. Error %d\n",
+				source->name, sink->name,
+				flags ? "enabled" : "disabled",
+				ret);
+			return ret;
+		} else
+			dev_dbg(fe->dvb->device,
+				"link %s->%s was %s\n",
+				source->name, sink->name,
+				flags ? "ENABLED" : "disabled");
+	}
+#endif
+	return 0;
+}
+
 static int dvb_frontend_thread(void *data)
 {
 	struct dvb_frontend *fe = data;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	fe_status_t s;
 	enum dvbfe_algo algo;
+	int ret;
 
 	bool re_tune = false;
 	bool semheld = false;
@@ -609,6 +696,13 @@ static int dvb_frontend_thread(void *data)
 	fepriv->wakeup = 0;
 	fepriv->reinitialise = 0;
 
+	ret = dvb_enable_media_tuner(fe);
+	if (ret) {
+		/* FIXME: return an error if it fails */
+		dev_info(fe->dvb->device,
+			"proceeding with FE task\n");
+	}
+
 	dvb_frontend_init(fe);
 
 	set_freezable();
-- 
2.1.0

[PATCHv4 24/25] [media] cx231xx: enable tuner->decoder link at videobuf start

[Mauro Carvalho Chehab]

The tuner->decoder needs to be enabled when we're about to
start streaming.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index f3d1a488dfa7..634763535d60 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -703,6 +703,74 @@ static void free_buffer(struct videobuf_queue *vq, struct cx231xx_buffer *buf)
 	buf->vb.state = VIDEOBUF_NEEDS_INIT;
 }
 
+static int cx231xx_enable_analog_tuner(struct cx231xx *dev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	struct media_device *mdev = dev->media_dev;
+	struct media_entity  *entity, *decoder = NULL, *source;
+	struct media_link *link, *found_link = NULL;
+	int i, ret, active_links = 0;
+
+	if (!mdev)
+		return 0;
+
+/*
+ * This will find the tuner that it is connected into the decoder.
+ * Technically, this is not 100% correct, as the device may be using an
+ * analog input instead of the tuner. However, we can't use the DVB for dvb
+ * while the DMA engine is being used for V4L2.
+ */
+	media_device_for_each_entity(entity, mdev) {
+		if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
+			decoder = entity;
+			break;
+		}
+	}
+	if (!decoder)
+		return 0;
+
+	for (i = 0; i < decoder->num_links; i++) {
+		link = &decoder->links[i];
+		if (link->sink->entity == decoder) {
+			found_link = link;
+			if (link->flags & MEDIA_LNK_FL_ENABLED)
+				active_links++;
+			break;
+		}
+	}
+
+	if (active_links == 1 || !found_link)
+		return 0;
+
+	source = found_link->source->entity;
+	for (i = 0; i < source->num_links; i++) {
+		struct media_entity *sink;
+		int flags = 0;
+
+		link = &source->links[i];
+		sink = link->sink->entity;
+
+		if (sink == entity)
+			flags = MEDIA_LNK_FL_ENABLED;
+
+		ret = media_entity_setup_link(link, flags);
+		if (ret) {
+			dev_err(dev->dev,
+				"Couldn't change link %s->%s to %s. Error %d\n",
+				source->name, sink->name,
+				flags ? "enabled" : "disabled",
+				ret);
+			return ret;
+		} else
+			dev_dbg(dev->dev,
+				"link %s->%s was %s\n",
+				source->name, sink->name,
+				flags ? "ENABLED" : "disabled");
+	}
+#endif
+	return 0;
+}
+
 static int
 buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
 	       enum v4l2_field field)
@@ -756,6 +824,9 @@ buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
 	}
 
 	buf->vb.state = VIDEOBUF_PREPARED;
+
+	cx231xx_enable_analog_tuner(dev);
+
 	return 0;
 
 fail:
-- 
2.1.0

Re: [PATCHv4 24/25] [media] cx231xx: enable tuner->decoder link at videobuf start

[Hans Verkuil]

On 02/13/2015 11:58 PM, Mauro Carvalho Chehab wrote:
> The tuner->decoder needs to be enabled when we're about to
> start streaming.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
> index f3d1a488dfa7..634763535d60 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-video.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-video.c
> @@ -703,6 +703,74 @@ static void free_buffer(struct videobuf_queue *vq, struct cx231xx_buffer *buf)
>  	buf->vb.state = VIDEOBUF_NEEDS_INIT;
>  }
>  
> +static int cx231xx_enable_analog_tuner(struct cx231xx *dev)
> +{
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	struct media_device *mdev = dev->media_dev;
> +	struct media_entity  *entity, *decoder = NULL, *source;
> +	struct media_link *link, *found_link = NULL;
> +	int i, ret, active_links = 0;
> +
> +	if (!mdev)
> +		return 0;
> +
> +/*
> + * This will find the tuner that it is connected into the decoder.
> + * Technically, this is not 100% correct, as the device may be using an
> + * analog input instead of the tuner. However, we can't use the DVB for dvb

'we can't use the DVB for dvb'?? You probably mean 'can't use the DVB API'.

> + * while the DMA engine is being used for V4L2.
> + */

Weird indentation, should be one to the right.

> +	media_device_for_each_entity(entity, mdev) {
> +		if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
> +			decoder = entity;
> +			break;
> +		}
> +	}
> +	if (!decoder)
> +		return 0;
> +
> +	for (i = 0; i < decoder->num_links; i++) {
> +		link = &decoder->links[i];
> +		if (link->sink->entity == decoder) {
> +			found_link = link;
> +			if (link->flags & MEDIA_LNK_FL_ENABLED)
> +				active_links++;
> +			break;
> +		}
> +	}
> +
> +	if (active_links == 1 || !found_link)
> +		return 0;
> +
> +	source = found_link->source->entity;
> +	for (i = 0; i < source->num_links; i++) {
> +		struct media_entity *sink;
> +		int flags = 0;
> +
> +		link = &source->links[i];
> +		sink = link->sink->entity;
> +
> +		if (sink == entity)
> +			flags = MEDIA_LNK_FL_ENABLED;
> +
> +		ret = media_entity_setup_link(link, flags);
> +		if (ret) {
> +			dev_err(dev->dev,
> +				"Couldn't change link %s->%s to %s. Error %d\n",
> +				source->name, sink->name,
> +				flags ? "enabled" : "disabled",
> +				ret);
> +			return ret;
> +		} else
> +			dev_dbg(dev->dev,
> +				"link %s->%s was %s\n",
> +				source->name, sink->name,
> +				flags ? "ENABLED" : "disabled");
> +	}
> +#endif
> +	return 0;
> +}
> +
>  static int
>  buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
>  	       enum v4l2_field field)
> @@ -756,6 +824,9 @@ buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
>  	}
>  
>  	buf->vb.state = VIDEOBUF_PREPARED;
> +
> +	cx231xx_enable_analog_tuner(dev);

Is this the right place? Isn't this now called for every QBUF? I would expect this
to happen when you start streaming.

In vb2 it would be in start_streaming(), of course.

Regards,

	Hans

> +
>  	return 0;
>  
>  fail:
> 

Regards,

	Hans

Re: [PATCHv4 24/25] [media] cx231xx: enable tuner->decoder link at videobuf start

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 10:27:15 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/13/2015 11:58 PM, Mauro Carvalho Chehab wrote:
> > The tuner->decoder needs to be enabled when we're about to
> > start streaming.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
> > index f3d1a488dfa7..634763535d60 100644
> > --- a/drivers/media/usb/cx231xx/cx231xx-video.c
> > +++ b/drivers/media/usb/cx231xx/cx231xx-video.c
> > @@ -703,6 +703,74 @@ static void free_buffer(struct videobuf_queue *vq, struct cx231xx_buffer *buf)
> >  	buf->vb.state = VIDEOBUF_NEEDS_INIT;
> >  }
> >  
> > +static int cx231xx_enable_analog_tuner(struct cx231xx *dev)
> > +{
> > +#ifdef CONFIG_MEDIA_CONTROLLER
> > +	struct media_device *mdev = dev->media_dev;
> > +	struct media_entity  *entity, *decoder = NULL, *source;
> > +	struct media_link *link, *found_link = NULL;
> > +	int i, ret, active_links = 0;
> > +
> > +	if (!mdev)
> > +		return 0;
> > +
> > +/*
> > + * This will find the tuner that it is connected into the decoder.
> > + * Technically, this is not 100% correct, as the device may be using an
> > + * analog input instead of the tuner. However, we can't use the DVB for dvb
> 
> 'we can't use the DVB for dvb'?? You probably mean 'can't use the DVB API'.

What I meant to say is that we can't use the device for DVB while it is
doing an analog stream.

I'll fix this on a separate patch.

> > + * while the DMA engine is being used for V4L2.
> > + */
> 
> Weird indentation, should be one to the right.

Ah, true. I'll fix this too.

> > +	media_device_for_each_entity(entity, mdev) {
> > +		if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
> > +			decoder = entity;
> > +			break;
> > +		}
> > +	}
> > +	if (!decoder)
> > +		return 0;
> > +
> > +	for (i = 0; i < decoder->num_links; i++) {
> > +		link = &decoder->links[i];
> > +		if (link->sink->entity == decoder) {
> > +			found_link = link;
> > +			if (link->flags & MEDIA_LNK_FL_ENABLED)
> > +				active_links++;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (active_links == 1 || !found_link)
> > +		return 0;
> > +
> > +	source = found_link->source->entity;
> > +	for (i = 0; i < source->num_links; i++) {
> > +		struct media_entity *sink;
> > +		int flags = 0;
> > +
> > +		link = &source->links[i];
> > +		sink = link->sink->entity;
> > +
> > +		if (sink == entity)
> > +			flags = MEDIA_LNK_FL_ENABLED;
> > +
> > +		ret = media_entity_setup_link(link, flags);
> > +		if (ret) {
> > +			dev_err(dev->dev,
> > +				"Couldn't change link %s->%s to %s. Error %d\n",
> > +				source->name, sink->name,
> > +				flags ? "enabled" : "disabled",
> > +				ret);
> > +			return ret;
> > +		} else
> > +			dev_dbg(dev->dev,
> > +				"link %s->%s was %s\n",
> > +				source->name, sink->name,
> > +				flags ? "ENABLED" : "disabled");
> > +	}
> > +#endif
> > +	return 0;
> > +}
> > +
> >  static int
> >  buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
> >  	       enum v4l2_field field)
> > @@ -756,6 +824,9 @@ buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
> >  	}
> >  
> >  	buf->vb.state = VIDEOBUF_PREPARED;
> > +
> > +	cx231xx_enable_analog_tuner(dev);
> 
> Is this the right place? Isn't this now called for every QBUF? I would expect this
> to happen when you start streaming.
> 
> In vb2 it would be in start_streaming(), of course.

True. I think that the vb1 dialog would be, instead, ops->buf_setup.
I'll fix this on a separate patch.

Regards,
Mauro

[PATCHv4 25/25] [media] dvb_frontend: start media pipeline while thread is running

[Mauro Carvalho Chehab]

While the DVB thread is running, the media pipeline should be
streaming. This should prevent any attempt of using the analog
TV while digital TV is working, and vice-versa.

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

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 50bc6056e914..aa5306908193 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -131,6 +131,11 @@ struct dvb_frontend_private {
 	int quality;
 	unsigned int check_wrapped;
 	enum dvbfe_search algo_status;
+
+#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
+	struct media_pipeline pipe;
+	struct media_entity *pipe_start_entity;
+#endif
 };
 
 static void dvb_frontend_wakeup(struct dvb_frontend *fe);
@@ -608,9 +613,9 @@ static void dvb_frontend_wakeup(struct dvb_frontend *fe)
  * or 0 if everything is OK, if no tuner is linked to the frontend or if the
  * mdev is NULL.
  */
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
 static int dvb_enable_media_tuner(struct dvb_frontend *fe)
 {
-#ifdef CONFIG_MEDIA_CONTROLLER_DVB
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	struct dvb_adapter *adapter = fe->dvb;
 	struct media_device *mdev = adapter->mdev;
@@ -618,10 +623,14 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
 	struct media_link *link, *found_link = NULL;
 	int i, ret, n_links = 0, active_links = 0;
 
+	fepriv->pipe_start_entity = NULL;
+
 	if (!mdev)
 		return 0;
 
 	entity = fepriv->dvbdev->entity;
+	fepriv->pipe_start_entity = entity;
+
 	for (i = 0; i < entity->num_links; i++) {
 		link = &entity->links[i];
 		if (link->sink->entity == entity) {
@@ -648,6 +657,7 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
 	}
 
 	source = found_link->source->entity;
+	fepriv->pipe_start_entity = source;
 	for (i = 0; i < source->num_links; i++) {
 		struct media_entity *sink;
 		int flags = 0;
@@ -672,9 +682,9 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
 				source->name, sink->name,
 				flags ? "ENABLED" : "disabled");
 	}
-#endif
 	return 0;
 }
+#endif
 
 static int dvb_frontend_thread(void *data)
 {
@@ -696,12 +706,19 @@ static int dvb_frontend_thread(void *data)
 	fepriv->wakeup = 0;
 	fepriv->reinitialise = 0;
 
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
 	ret = dvb_enable_media_tuner(fe);
 	if (ret) {
 		/* FIXME: return an error if it fails */
 		dev_info(fe->dvb->device,
 			"proceeding with FE task\n");
+	} else {
+		ret = media_entity_pipeline_start(fepriv->pipe_start_entity,
+						  &fepriv->pipe);
+		if (ret)
+			return ret;
 	}
+#endif
 
 	dvb_frontend_init(fe);
 
@@ -812,6 +829,11 @@ restart:
 		}
 	}
 
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+	media_entity_pipeline_stop(fepriv->pipe_start_entity);
+	fepriv->pipe_start_entity = NULL;
+#endif
+
 	if (dvb_powerdown_on_sleep) {
 		if (fe->ops.set_voltage)
 			fe->ops.set_voltage(fe, SEC_VOLTAGE_OFF);
-- 
2.1.0

Re: [PATCHv4 25/25] [media] dvb_frontend: start media pipeline while thread is running

[Hans Verkuil]

On 02/13/2015 11:58 PM, Mauro Carvalho Chehab wrote:
> While the DVB thread is running, the media pipeline should be
> streaming. This should prevent any attempt of using the analog
> TV while digital TV is working, and vice-versa.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index 50bc6056e914..aa5306908193 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -131,6 +131,11 @@ struct dvb_frontend_private {
>  	int quality;
>  	unsigned int check_wrapped;
>  	enum dvbfe_search algo_status;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> +	struct media_pipeline pipe;
> +	struct media_entity *pipe_start_entity;
> +#endif
>  };
>  
>  static void dvb_frontend_wakeup(struct dvb_frontend *fe);
> @@ -608,9 +613,9 @@ static void dvb_frontend_wakeup(struct dvb_frontend *fe)
>   * or 0 if everything is OK, if no tuner is linked to the frontend or if the
>   * mdev is NULL.
>   */
> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
>  static int dvb_enable_media_tuner(struct dvb_frontend *fe)
>  {
> -#ifdef CONFIG_MEDIA_CONTROLLER_DVB
>  	struct dvb_frontend_private *fepriv = fe->frontend_priv;
>  	struct dvb_adapter *adapter = fe->dvb;
>  	struct media_device *mdev = adapter->mdev;
> @@ -618,10 +623,14 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
>  	struct media_link *link, *found_link = NULL;
>  	int i, ret, n_links = 0, active_links = 0;
>  
> +	fepriv->pipe_start_entity = NULL;
> +
>  	if (!mdev)
>  		return 0;
>  
>  	entity = fepriv->dvbdev->entity;
> +	fepriv->pipe_start_entity = entity;
> +
>  	for (i = 0; i < entity->num_links; i++) {
>  		link = &entity->links[i];
>  		if (link->sink->entity == entity) {
> @@ -648,6 +657,7 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
>  	}
>  
>  	source = found_link->source->entity;
> +	fepriv->pipe_start_entity = source;
>  	for (i = 0; i < source->num_links; i++) {
>  		struct media_entity *sink;
>  		int flags = 0;
> @@ -672,9 +682,9 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
>  				source->name, sink->name,
>  				flags ? "ENABLED" : "disabled");
>  	}
> -#endif
>  	return 0;
>  }
> +#endif
>  
>  static int dvb_frontend_thread(void *data)
>  {
> @@ -696,12 +706,19 @@ static int dvb_frontend_thread(void *data)
>  	fepriv->wakeup = 0;
>  	fepriv->reinitialise = 0;
>  
> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB

With this change I get this warning:

drivers/media/dvb-core/dvb_frontend.c: In function ‘dvb_frontend_thread’:
drivers/media/dvb-core/dvb_frontend.c:695:6: warning: unused variable ‘ret’ [-Wunused-variable]
  int ret;
      ^

So the 'ret' variable declaration should be under #ifdef CONFIG_MEDIA_CONTROLLER_DVB
as well.

Regards,

	Hans

>  	ret = dvb_enable_media_tuner(fe);
>  	if (ret) {
>  		/* FIXME: return an error if it fails */
>  		dev_info(fe->dvb->device,
>  			"proceeding with FE task\n");
> +	} else {
> +		ret = media_entity_pipeline_start(fepriv->pipe_start_entity,
> +						  &fepriv->pipe);
> +		if (ret)
> +			return ret;
>  	}
> +#endif
>  
>  	dvb_frontend_init(fe);
>  
> @@ -812,6 +829,11 @@ restart:
>  		}
>  	}
>  
> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
> +	media_entity_pipeline_stop(fepriv->pipe_start_entity);
> +	fepriv->pipe_start_entity = NULL;
> +#endif
> +
>  	if (dvb_powerdown_on_sleep) {
>  		if (fe->ops.set_voltage)
>  			fe->ops.set_voltage(fe, SEC_VOLTAGE_OFF);
> 

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Hans Verkuil]

On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> This patch series adds basic support for the media controller at the
> DVB core: it creates one media entity per DVB devnode, if the media
> device is passed as an argument to the DVB structures.
> 
> The cx231xx driver was modified to pass such argument for DVB NET,
> DVB frontend and DVB demux.
> 
> -
> 
> version 4:
> 
> - Addressed the issues pointed via e-mail

No, you didn't. Especially with regards to the alsa node definition. I'm
pretty sure you need at least the subdevice information which is now removed.

I also do *not* like the fact that you posted a v4 and immediately applied
these patches to the master without leaving any time for more discussions.

These patches change the kernel API and need to go to proper review and need
a bunch of Acks, Laurent's at the very minimum since he's MC maintainer.

Please revert the whole patch series from master, then we can discuss this
more.

For the record, for patch 02/25:

Nacked-by: Hans Verkuil <hans.verkuil@cisco.com>

I do *not* agree with this API change.

We can discuss this more on Monday.

Regards,

	Hans

> - Added a separate Kconfig option to enable media controller DVB
>   experimental support
> - Fixed some CodingStyle issues
> - Added documentation for the API changes at the DocBook
> 
> version 3:
> - Added the second series of patches ("add link graph to cx231xx 
>   using the media controller")
> - tuner-core and cx25840: add proper error handling as suggested by
>   Sakari Ailus and pointed by Joe Perches;
> - dvb core: move the media_dev struct to be inside the DVB adapter. That
>   allowed to simplify the changes for the dvbdev clients;
> - Add logic to setup the pipelines when analog or digital TV stream starts.
> - Renamed some patches to better describe its contents.
> 
> version 2:
> - Now the PADs are created for all nodes
> - Instead of using entity->flags for subtypes, create separate
>   MEDIA_ENT_T_DEVNODE_DVB_foo for each DVB devtype
> - The API change patch was split from the DVB core changes
> 
> Mauro Carvalho Chehab (24):
>   [media] media: Fix DVB devnode representation at media controller
>   [media] Docbook: Fix documentation for media controller devnodes
>   [media] media: add new types for DVB devnodes
>   [media] DocBook: Document the DVB API devnodes at the media controller
>   [media] media: add a subdev type for tuner
>   [media] DocBook: Add tuner subdev at documentation
>   [media] dvbdev: add support for media controller
>   [media] cx231xx: add media controller support
>   [media] dvb_frontend: add media controller support for DVB frontend
>   [media] dmxdev: add support for demux/dvr nodes at media controller
>   [media] dvb_ca_en50221: add support for CA node at the media
>     controller
>   [media] dvb_net: add support for DVB net node at the media controller
>   [media] dvbdev: add pad for the DVB devnodes
>   [media] tuner-core: properly initialize media controller subdev
>   [media] cx25840: fill the media controller entity
>   [media] cx231xx: initialize video/vbi pads
>   [media] cx231xx: create media links for analog mode
>   [media] dvbdev: represent frontend with two pads
>   [media] dvbdev: add a function to create DVB media graph
>   [media] cx231xx: create DVB graph
>   [media] dvbdev: enable DVB-specific links
>   [media] dvb-frontend: enable tuner link when the FE thread starts
>   [media] cx231xx: enable tuner->decoder link at videobuf start
>   [media] dvb_frontend: start media pipeline while thread is running
> 
> Zhangfei Gao (1):
>   [media] ir-hix5hd2: remove writel/readl_relaxed define
> 
>  .../DocBook/media/v4l/media-ioc-enum-entities.xml  | 102 ++++-----------
>  Documentation/DocBook/media/v4l/v4l2.xml           |   9 ++
>  drivers/media/Kconfig                              |  10 +-
>  drivers/media/dvb-core/dmxdev.c                    |  11 +-
>  drivers/media/dvb-core/dvb_ca_en50221.c            |   6 +-
>  drivers/media/dvb-core/dvb_frontend.c              | 121 ++++++++++++++++-
>  drivers/media/dvb-core/dvb_net.c                   |   6 +-
>  drivers/media/dvb-core/dvbdev.c                    | 143 ++++++++++++++++++++-
>  drivers/media/dvb-core/dvbdev.h                    |  15 +++
>  drivers/media/i2c/cx25840/cx25840-core.c           |  18 +++
>  drivers/media/i2c/cx25840/cx25840-core.h           |   3 +
>  drivers/media/rc/ir-hix5hd2.c                      |   8 --
>  drivers/media/usb/cx231xx/cx231xx-cards.c          |  98 +++++++++++++-
>  drivers/media/usb/cx231xx/cx231xx-dvb.c            |   5 +
>  drivers/media/usb/cx231xx/cx231xx-video.c          |  84 +++++++++++-
>  drivers/media/usb/cx231xx/cx231xx.h                |   5 +
>  drivers/media/v4l2-core/tuner-core.c               |  20 +++
>  drivers/media/v4l2-core/v4l2-dev.c                 |   4 +-
>  drivers/media/v4l2-core/v4l2-device.c              |   4 +-
>  include/media/media-entity.h                       |  12 +-
>  include/uapi/linux/media.h                         |  26 +++-
>  21 files changed, 592 insertions(+), 118 deletions(-)
> 

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Mauro Carvalho Chehab]

Em Sat, 14 Feb 2015 10:32:21 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> > This patch series adds basic support for the media controller at the
> > DVB core: it creates one media entity per DVB devnode, if the media
> > device is passed as an argument to the DVB structures.
> > 
> > The cx231xx driver was modified to pass such argument for DVB NET,
> > DVB frontend and DVB demux.
> > 
> > -
> > 
> > version 4:
> > 
> > - Addressed the issues pointed via e-mail
> 
> No, you didn't. Especially with regards to the alsa node definition. I'm
> pretty sure you need at least the subdevice information which is now removed.

Well, back on Jan, 26 I answered your issues about that at:
	http://www.spinics.net/lists/linux-media/msg85857.html

As you didn't reply back in a reasonable amount of time, I assumed that
you're happy with that.

In any case, the definitions are still there, as nothing got dropped
from the external header.

So, when ALSA media controller support will be added at the Kernel, we
can decide if it will use major/minor or card/device/subdevice or both.

As I said back in Jan, 26, IMO, the best would be to use both:

struct media_entity_desc {
	...
	union {
		struct {
			u32 major;
			u32 minor;
		} dev;
		/* deprecated fields */
		...
	}
	union {
		struct {
			u32 card;
			u32 device;
			u32 subdevice;
		} alsa_props;
		__u8 raw[172];
	}
}

(additional and deprecated fields removed just to simplify its
 representation above)

Even for ALSA, it is a way easier for libmediactl.c to keep using
major/minor to get the device node name via both udev/sysfs than
using anything else, as I don't think that udev has any method to
find the associated name without major,minor information. Ok, there
are indirect methods using the ALSA API to get such association, but
it is just easier to fill everything at the struct than to add the
extra complexity for the media control clients to convert between
major/minor into card/device/subdevice.

What I'm saying is that the card/device/subdevice really seems to be
an extra property for this specific type of devnode, and not a
replacement.

In any case, I think we should take the decision on how to properly
map the ALSA specific bits when we merge ALSA media controller patches,
and not before.

> I also do *not* like the fact that you posted a v4 and immediately applied
> these patches to the master without leaving any time for more discussions.
> 
> These patches change the kernel API and need to go to proper review and need
> a bunch of Acks, Laurent's at the very minimum since he's MC maintainer.
>
> Please revert the whole patch series from master, then we can discuss this
> more.
> 
> For the record, for patch 02/25:
> 
> Nacked-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> I do *not* agree with this API change.
> 
> We can discuss this more on Monday.

This hole series is for discussions for a long time (since the beginning of
January), without rejection, and its now starting to receive patches from
other authors. Keeping it OOT just makes harder to discuss and for people to
test. It is time to move on.

As I said on IRC, as I opted to merge it for 3.21, we'll have 2 entire
Kernel cycles to make it mature before being merged upstream. During that
period, we can fix any issues on it.

Regards,
Mauro

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Hans Verkuil]

On 02/14/2015 12:00 PM, Mauro Carvalho Chehab wrote:
> Em Sat, 14 Feb 2015 10:32:21 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
>>> This patch series adds basic support for the media controller at the
>>> DVB core: it creates one media entity per DVB devnode, if the media
>>> device is passed as an argument to the DVB structures.
>>>
>>> The cx231xx driver was modified to pass such argument for DVB NET,
>>> DVB frontend and DVB demux.
>>>
>>> -
>>>
>>> version 4:
>>>
>>> - Addressed the issues pointed via e-mail
>>
>> No, you didn't. Especially with regards to the alsa node definition. I'm
>> pretty sure you need at least the subdevice information which is now removed.
> 
> Well, back on Jan, 26 I answered your issues about that at:
> 	http://www.spinics.net/lists/linux-media/msg85857.html
> 
> As you didn't reply back in a reasonable amount of time, I assumed that
> you're happy with that.
> 
> In any case, the definitions are still there, as nothing got dropped
> from the external header.
> 
> So, when ALSA media controller support will be added at the Kernel, we
> can decide if it will use major/minor or card/device/subdevice or both.
> 
> As I said back in Jan, 26, IMO, the best would be to use both:
> 
> struct media_entity_desc {
> 	...
> 	union {
> 		struct {
> 			u32 major;
> 			u32 minor;
> 		} dev;
> 		/* deprecated fields */
> 		...
> 	}
> 	union {
> 		struct {
> 			u32 card;
> 			u32 device;
> 			u32 subdevice;
> 		} alsa_props;
> 		__u8 raw[172];
> 	}
> }
> 
> (additional and deprecated fields removed just to simplify its
>  representation above)
> 
> Even for ALSA, it is a way easier for libmediactl.c to keep using
> major/minor to get the device node name via both udev/sysfs than
> using anything else, as I don't think that udev has any method to
> find the associated name without major,minor information. Ok, there
> are indirect methods using the ALSA API to get such association, but
> it is just easier to fill everything at the struct than to add the
> extra complexity for the media control clients to convert between
> major/minor into card/device/subdevice.
> 
> What I'm saying is that the card/device/subdevice really seems to be
> an extra property for this specific type of devnode, and not a
> replacement.
> 
> In any case, I think we should take the decision on how to properly
> map the ALSA specific bits when we merge ALSA media controller patches,
> and not before.
> 
>> I also do *not* like the fact that you posted a v4 and immediately applied
>> these patches to the master without leaving any time for more discussions.
>>
>> These patches change the kernel API and need to go to proper review and need
>> a bunch of Acks, Laurent's at the very minimum since he's MC maintainer.
>>
>> Please revert the whole patch series from master, then we can discuss this
>> more.
>>
>> For the record, for patch 02/25:
>>
>> Nacked-by: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> I do *not* agree with this API change.
>>
>> We can discuss this more on Monday.
> 
> This hole series is for discussions for a long time (since the beginning of
> January), without rejection, and its now starting to receive patches from
> other authors. Keeping it OOT just makes harder to discuss and for people to
> test. It is time to move on.

No, it isn't. Laurent and myself actually discussed it during FOSDEM, and
we did have concerns about the API changes. Since he's the MC maintainer I
assumed he would get back to you, but I think he's been very busy since FOSDEM.
In fact, I believe he attended the Linaro Connect last week, so it is very
likely he will not have had time to do anything with what we discussed at
FOSDEM.

Patches changing the MC API need his Ack at minimum. You can't just post a
v4 patch series and commit the same day. Not when it changes APIs and especially
not since there were concerns raised about it in the past. You should have
pinged Laurent and probably myself and ask for comments on v4.

> 
> As I said on IRC, as I opted to merge it for 3.21, we'll have 2 entire
> Kernel cycles to make it mature before being merged upstream. During that
> period, we can fix any issues on it.

That's not the way things are supposed to work. You wouldn't merge patches
from us in a similar situation, and quite rightly. I know, we've tried :-)

Just revert, try to setup an irc session next week, based on that post a v5
and you are likely able to merge this within 2 weeks. Everyone agrees with
*what* you want to do, just not about some of the details.

Regards,

	Hans

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Mauro Carvalho Chehab]

Em Sat, 14 Feb 2015 12:43:30 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/14/2015 12:00 PM, Mauro Carvalho Chehab wrote:
> > Em Sat, 14 Feb 2015 10:32:21 +0100
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > 
> >> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> >>> This patch series adds basic support for the media controller at the
> >>> DVB core: it creates one media entity per DVB devnode, if the media
> >>> device is passed as an argument to the DVB structures.
> >>>
> >>> The cx231xx driver was modified to pass such argument for DVB NET,
> >>> DVB frontend and DVB demux.
> >>>
> >>> -
> >>>
> >>> version 4:
> >>>
> >>> - Addressed the issues pointed via e-mail
> >>
> >> No, you didn't. Especially with regards to the alsa node definition. I'm
> >> pretty sure you need at least the subdevice information which is now removed.
> > 
> > Well, back on Jan, 26 I answered your issues about that at:
> > 	http://www.spinics.net/lists/linux-media/msg85857.html
> > 
> > As you didn't reply back in a reasonable amount of time, I assumed that
> > you're happy with that.
> > 
> > In any case, the definitions are still there, as nothing got dropped
> > from the external header.
> > 
> > So, when ALSA media controller support will be added at the Kernel, we
> > can decide if it will use major/minor or card/device/subdevice or both.
> > 
> > As I said back in Jan, 26, IMO, the best would be to use both:
> > 
> > struct media_entity_desc {
> > 	...
> > 	union {
> > 		struct {
> > 			u32 major;
> > 			u32 minor;
> > 		} dev;
> > 		/* deprecated fields */
> > 		...
> > 	}
> > 	union {
> > 		struct {
> > 			u32 card;
> > 			u32 device;
> > 			u32 subdevice;
> > 		} alsa_props;
> > 		__u8 raw[172];
> > 	}
> > }
> > 
> > (additional and deprecated fields removed just to simplify its
> >  representation above)
> > 
> > Even for ALSA, it is a way easier for libmediactl.c to keep using
> > major/minor to get the device node name via both udev/sysfs than
> > using anything else, as I don't think that udev has any method to
> > find the associated name without major,minor information. Ok, there
> > are indirect methods using the ALSA API to get such association, but
> > it is just easier to fill everything at the struct than to add the
> > extra complexity for the media control clients to convert between
> > major/minor into card/device/subdevice.
> > 
> > What I'm saying is that the card/device/subdevice really seems to be
> > an extra property for this specific type of devnode, and not a
> > replacement.
> > 
> > In any case, I think we should take the decision on how to properly
> > map the ALSA specific bits when we merge ALSA media controller patches,
> > and not before.
> > 
> >> I also do *not* like the fact that you posted a v4 and immediately applied
> >> these patches to the master without leaving any time for more discussions.
> >>
> >> These patches change the kernel API and need to go to proper review and need
> >> a bunch of Acks, Laurent's at the very minimum since he's MC maintainer.
> >>
> >> Please revert the whole patch series from master, then we can discuss this
> >> more.
> >>
> >> For the record, for patch 02/25:
> >>
> >> Nacked-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>
> >> I do *not* agree with this API change.
> >>
> >> We can discuss this more on Monday.
> > 
> > This hole series is for discussions for a long time (since the beginning of
> > January), without rejection, and its now starting to receive patches from
> > other authors. Keeping it OOT just makes harder to discuss and for people to
> > test. It is time to move on.
> 
> No, it isn't. Laurent and myself actually discussed it during FOSDEM, and
> we did have concerns about the API changes. Since he's the MC maintainer I
> assumed he would get back to you, but I think he's been very busy since FOSDEM.
> In fact, I believe he attended the Linaro Connect last week, so it is very
> likely he will not have had time to do anything with what we discussed at
> FOSDEM.

No, I hadn't any return from that, nor by e-mail or IRC, except for Devin's
emails saying he's ok, provided that we'll also look at the cases with
multiple DVB adapters. I have already some patches covering it, but, in
order to discuss, it is important to merge the work I have already done,
plus the patches from Rafael, as my patches are in the top of his ones.

> Patches changing the MC API need his Ack at minimum. You can't just post a
> v4 patch series and commit the same day. Not when it changes APIs and especially
> not since there were concerns raised about it in the past. You should have
> pinged Laurent and probably myself and ask for comments on v4.

My understanding from the last discussions on IRC with Laurent is that, while
he prefers to not get this merged for 3.20, he is ok if I do that. So, I
took the decision of waiting till the merge of the patches for 3.20 to be
merged and adding those patches for 3.21, giving us 2 Kernel cycles to
do a deeper review. 

This is basically the same thing I do with all API changes: try to merge
them before -rc1 (or at early -rc) at our development tree, in order to
let all of us to test and to fix needed things before it is too late.

> > 
> > As I said on IRC, as I opted to merge it for 3.21, we'll have 2 entire
> > Kernel cycles to make it mature before being merged upstream. During that
> > period, we can fix any issues on it.
> 
> That's not the way things are supposed to work. You wouldn't merge patches
> from us in a similar situation, and quite rightly. I know, we've tried :-)

No, we did fix API bits on stuff already merged on our development tree.
More than that, we did changes even after merging them upstream, during
-rc Kernels, including API variable renames. Several of such changes were
even done or requested by you ;)

One such example is changeset 87185c958de9cd4acd8392f00d6161f4e11807ff
wich was changed at v3.14-rc5-379-g87185c958de9.

For the record, the issue we're suffering with the MC is basically because
we didn't follow the proper procedure when MC got merged. 

The golden rule is that we don't add UAPIs at the Kernel without any
driver using it. However, for MC, we added support for DVB, FB and ALSA
before having any driver using.

That is the source of the problems we're needing to fix now, as just
patching the old structures could cause media controller userspace
applications to not build, so we're forced to deprecate the earlier
bad design decisions, in favor to something that would actually work.

Thankfully, FB is doing the right thing: using major/minor for devnodes,
but ALSA and DVB designs took a risky path.

In the case of DVB, there are as just an "ID" is meaningless there, as
a DVB device needs at least an adapter ID plus a "type" ID in order
to uniquely specify a device instance. For example, the DVB demux
devnodes are /dev/dvb/adapter?/demux?, if there's no udev rule renaming
it to something else.

The big issue of using an adapter_id/type_id, however, and the same applies
to ALSA using card/device/subdevice, is that udev may have rules renaming
the device node to something else. As udev knows nothing about DVB (or
ALSA or whatever), we need to get the device's major/minor, in order to
check what's the name of the devnode he actually created.

So, as properly pointed inside the media-ctl source code, the right and
direct way  to get the path name associated with a device node is to use
its major/minor and ask libudev about what's the path associated with
that given device.

In any case, for ALSA, we should do the right thing here: remove (actually
deprecate) whatever definition is there, and then re-add it only when we
actually have the patches inside the ALSA subsystem to support the media
controller, plus having the corresponding patches for the media-ctl in order
to support the devnode discovery using both udev and sysfs for their nodes.

> Just revert, try to setup an irc session next week, based on that post a v5
> and you are likely able to merge this within 2 weeks. Everyone agrees with
> *what* you want to do, just not about some of the details.

I'm in a holiday this week up to Tuesday. Even if I want, I can't revert
it right now. Let's schedule a meeting on Wed via IRC, in order to discuss
it.

In the mean time, if you have any patches that reflecting the approach
you think it is more appropriate, please feel free to send it to ML
for discussions. I'll try to take a look at my mailbox from time to time
during this holiday.

Regards,
Mauro

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Hans Verkuil]

On 02/15/2015 11:27 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 14 Feb 2015 12:43:30 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 02/14/2015 12:00 PM, Mauro Carvalho Chehab wrote:
>>> Em Sat, 14 Feb 2015 10:32:21 +0100
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>
>>>> On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
>>>>> This patch series adds basic support for the media controller at the
>>>>> DVB core: it creates one media entity per DVB devnode, if the media
>>>>> device is passed as an argument to the DVB structures.
>>>>>
>>>>> The cx231xx driver was modified to pass such argument for DVB NET,
>>>>> DVB frontend and DVB demux.
>>>>>
>>>>> -
>>>>>
>>>>> version 4:
>>>>>
>>>>> - Addressed the issues pointed via e-mail
>>>>
>>>> No, you didn't. Especially with regards to the alsa node definition. I'm
>>>> pretty sure you need at least the subdevice information which is now removed.
>>>
>>> Well, back on Jan, 26 I answered your issues about that at:
>>> 	http://www.spinics.net/lists/linux-media/msg85857.html
>>>
>>> As you didn't reply back in a reasonable amount of time, I assumed that
>>> you're happy with that.
>>>
>>> In any case, the definitions are still there, as nothing got dropped
>>> from the external header.
>>>
>>> So, when ALSA media controller support will be added at the Kernel, we
>>> can decide if it will use major/minor or card/device/subdevice or both.
>>>
>>> As I said back in Jan, 26, IMO, the best would be to use both:
>>>
>>> struct media_entity_desc {
>>> 	...
>>> 	union {
>>> 		struct {
>>> 			u32 major;
>>> 			u32 minor;
>>> 		} dev;
>>> 		/* deprecated fields */
>>> 		...
>>> 	}
>>> 	union {
>>> 		struct {
>>> 			u32 card;
>>> 			u32 device;
>>> 			u32 subdevice;
>>> 		} alsa_props;
>>> 		__u8 raw[172];
>>> 	}
>>> }
>>>
>>> (additional and deprecated fields removed just to simplify its
>>>  representation above)
>>>
>>> Even for ALSA, it is a way easier for libmediactl.c to keep using
>>> major/minor to get the device node name via both udev/sysfs than
>>> using anything else, as I don't think that udev has any method to
>>> find the associated name without major,minor information. Ok, there
>>> are indirect methods using the ALSA API to get such association, but
>>> it is just easier to fill everything at the struct than to add the
>>> extra complexity for the media control clients to convert between
>>> major/minor into card/device/subdevice.
>>>
>>> What I'm saying is that the card/device/subdevice really seems to be
>>> an extra property for this specific type of devnode, and not a
>>> replacement.
>>>
>>> In any case, I think we should take the decision on how to properly
>>> map the ALSA specific bits when we merge ALSA media controller patches,
>>> and not before.
>>>
>>>> I also do *not* like the fact that you posted a v4 and immediately applied
>>>> these patches to the master without leaving any time for more discussions.
>>>>
>>>> These patches change the kernel API and need to go to proper review and need
>>>> a bunch of Acks, Laurent's at the very minimum since he's MC maintainer.
>>>>
>>>> Please revert the whole patch series from master, then we can discuss this
>>>> more.
>>>>
>>>> For the record, for patch 02/25:
>>>>
>>>> Nacked-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> I do *not* agree with this API change.
>>>>
>>>> We can discuss this more on Monday.
>>>
>>> This hole series is for discussions for a long time (since the beginning of
>>> January), without rejection, and its now starting to receive patches from
>>> other authors. Keeping it OOT just makes harder to discuss and for people to
>>> test. It is time to move on.
>>
>> No, it isn't. Laurent and myself actually discussed it during FOSDEM, and
>> we did have concerns about the API changes. Since he's the MC maintainer I
>> assumed he would get back to you, but I think he's been very busy since FOSDEM.
>> In fact, I believe he attended the Linaro Connect last week, so it is very
>> likely he will not have had time to do anything with what we discussed at
>> FOSDEM.
> 
> No, I hadn't any return from that, nor by e-mail or IRC, except for Devin's
> emails saying he's ok, provided that we'll also look at the cases with
> multiple DVB adapters. I have already some patches covering it, but, in
> order to discuss, it is important to merge the work I have already done,
> plus the patches from Rafael, as my patches are in the top of his ones.
> 
>> Patches changing the MC API need his Ack at minimum. You can't just post a
>> v4 patch series and commit the same day. Not when it changes APIs and especially
>> not since there were concerns raised about it in the past. You should have
>> pinged Laurent and probably myself and ask for comments on v4.
> 
> My understanding from the last discussions on IRC with Laurent is that, while
> he prefers to not get this merged for 3.20, he is ok if I do that. So, I
> took the decision of waiting till the merge of the patches for 3.20 to be
> merged and adding those patches for 3.21, giving us 2 Kernel cycles to
> do a deeper review. 
> 
> This is basically the same thing I do with all API changes: try to merge
> them before -rc1 (or at early -rc) at our development tree, in order to
> let all of us to test and to fix needed things before it is too late.
> 
>>>
>>> As I said on IRC, as I opted to merge it for 3.21, we'll have 2 entire
>>> Kernel cycles to make it mature before being merged upstream. During that
>>> period, we can fix any issues on it.
>>
>> That's not the way things are supposed to work. You wouldn't merge patches
>> from us in a similar situation, and quite rightly. I know, we've tried :-)
> 
> No, we did fix API bits on stuff already merged on our development tree.
> More than that, we did changes even after merging them upstream, during
> -rc Kernels, including API variable renames. Several of such changes were
> even done or requested by you ;)

Of course, if we realize we missed something, then we change it. But when these
API changes were merged, it had our acks.

The point is that a post-and-commit is simply bad form, especially since the
post contained new patches not posted before, and especially if it contains
API changes.

Occasionally we do a post-and-commit, but that's usually reserved for fixing
compiler warnings or stupid bugs/regressions that we missed and that need to
be addressed urgently.

> One such example is changeset 87185c958de9cd4acd8392f00d6161f4e11807ff
> wich was changed at v3.14-rc5-379-g87185c958de9.
> 
> For the record, the issue we're suffering with the MC is basically because
> we didn't follow the proper procedure when MC got merged. 
> 
> The golden rule is that we don't add UAPIs at the Kernel without any
> driver using it. However, for MC, we added support for DVB, FB and ALSA
> before having any driver using.
> 
> That is the source of the problems we're needing to fix now, as just
> patching the old structures could cause media controller userspace
> applications to not build, so we're forced to deprecate the earlier
> bad design decisions, in favor to something that would actually work.

I absolutely agree that we should never have added dvb/fb/alsa support at
that time. That was stupid in hindsight.

> Thankfully, FB is doing the right thing: using major/minor for devnodes,
> but ALSA and DVB designs took a risky path.
> 
> In the case of DVB, there are as just an "ID" is meaningless there, as
> a DVB device needs at least an adapter ID plus a "type" ID in order
> to uniquely specify a device instance. For example, the DVB demux
> devnodes are /dev/dvb/adapter?/demux?, if there's no udev rule renaming
> it to something else.
> 
> The big issue of using an adapter_id/type_id, however, and the same applies
> to ALSA using card/device/subdevice, is that udev may have rules renaming
> the device node to something else. As udev knows nothing about DVB (or
> ALSA or whatever), we need to get the device's major/minor, in order to
> check what's the name of the devnode he actually created.
> 
> So, as properly pointed inside the media-ctl source code, the right and
> direct way  to get the path name associated with a device node is to use
> its major/minor and ask libudev about what's the path associated with
> that given device.
> 
> In any case, for ALSA, we should do the right thing here: remove (actually
> deprecate) whatever definition is there, and then re-add it only when we
> actually have the patches inside the ALSA subsystem to support the media
> controller, plus having the corresponding patches for the media-ctl in order
> to support the devnode discovery using both udev and sysfs for their nodes.

I actually thought about how alsa should be handled and it is doing the
right thing. See my patch that I posted today, partially reverting your
patch.

> 
>> Just revert, try to setup an irc session next week, based on that post a v5
>> and you are likely able to merge this within 2 weeks. Everyone agrees with
>> *what* you want to do, just not about some of the details.
> 
> I'm in a holiday this week up to Tuesday. Even if I want, I can't revert
> it right now. Let's schedule a meeting on Wed via IRC, in order to discuss
> it.
> 
> In the mean time, if you have any patches that reflecting the approach
> you think it is more appropriate, please feel free to send it to ML
> for discussions. I'll try to take a look at my mailbox from time to time
> during this holiday.

Done.

Regards,

	Hans

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Mauro Carvalho Chehab]

Em Mon, 16 Feb 2015 10:55:50 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/15/2015 11:27 AM, Mauro Carvalho Chehab wrote:

> > In any case, for ALSA, we should do the right thing here: remove (actually
> > deprecate) whatever definition is there, and then re-add it only when we
> > actually have the patches inside the ALSA subsystem to support the media
> > controller, plus having the corresponding patches for the media-ctl in order
> > to support the devnode discovery using both udev and sysfs for their nodes.
> 
> I actually thought about how alsa should be handled and it is doing the
> right thing. See my patch that I posted today, partially reverting your
> patch.

Well, I can live with that patch for now, but I suspect that removing
major/minor from ALSA will make things very complex at the userspace side.

Do you know how to convert from card/device/subdevice into a device node
patch using libudev and sysfs?

Regards,
Mauro

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Hans Verkuil]

On 02/16/2015 11:50 AM, Mauro Carvalho Chehab wrote:
> Em Mon, 16 Feb 2015 10:55:50 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 02/15/2015 11:27 AM, Mauro Carvalho Chehab wrote:
> 
>>> In any case, for ALSA, we should do the right thing here: remove (actually
>>> deprecate) whatever definition is there, and then re-add it only when we
>>> actually have the patches inside the ALSA subsystem to support the media
>>> controller, plus having the corresponding patches for the media-ctl in order
>>> to support the devnode discovery using both udev and sysfs for their nodes.
>>
>> I actually thought about how alsa should be handled and it is doing the
>> right thing. See my patch that I posted today, partially reverting your
>> patch.
> 
> Well, I can live with that patch for now, but I suspect that removing
> major/minor from ALSA will make things very complex at the userspace side.
> 
> Do you know how to convert from card/device/subdevice into a device node
> patch using libudev and sysfs?

You don't *want* a device. The user space alsa utilities all operate on
card/device/subdevice level. I've never used an alsa device node directly,
or seen anyone do that. That's all done by the alsa library.

But regardless of whether or not it is right or wrong, as long as we do not
actually implement this in a driver I would keep this and wait until we
do have a working example. Replacing the alsa struct by a simple major/minor
is certainly not enough, of that I am 100% certain.

Let's keep it as is, and only touch it again when we actually get the first
user.

Regards,

	Hans

Re: [PATCHv4 00/25] dvb core: add basic support for the media controller

[Hans Verkuil]

On 02/13/2015 11:57 PM, Mauro Carvalho Chehab wrote:
> This patch series adds basic support for the media controller at the
> DVB core: it creates one media entity per DVB devnode, if the media
> device is passed as an argument to the DVB structures.
> 
> The cx231xx driver was modified to pass such argument for DVB NET,
> DVB frontend and DVB demux.

I've posted review comments for several patches and a new patch partially
reverting patches 2 and 4.

For the other patches:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> 
> -
> 
> version 4:
> 
> - Addressed the issues pointed via e-mail
> - Added a separate Kconfig option to enable media controller DVB
>   experimental support
> - Fixed some CodingStyle issues
> - Added documentation for the API changes at the DocBook
> 
> version 3:
> - Added the second series of patches ("add link graph to cx231xx 
>   using the media controller")
> - tuner-core and cx25840: add proper error handling as suggested by
>   Sakari Ailus and pointed by Joe Perches;
> - dvb core: move the media_dev struct to be inside the DVB adapter. That
>   allowed to simplify the changes for the dvbdev clients;
> - Add logic to setup the pipelines when analog or digital TV stream starts.
> - Renamed some patches to better describe its contents.
> 
> version 2:
> - Now the PADs are created for all nodes
> - Instead of using entity->flags for subtypes, create separate
>   MEDIA_ENT_T_DEVNODE_DVB_foo for each DVB devtype
> - The API change patch was split from the DVB core changes
> 
> Mauro Carvalho Chehab (24):
>   [media] media: Fix DVB devnode representation at media controller
>   [media] Docbook: Fix documentation for media controller devnodes
>   [media] media: add new types for DVB devnodes
>   [media] DocBook: Document the DVB API devnodes at the media controller
>   [media] media: add a subdev type for tuner
>   [media] DocBook: Add tuner subdev at documentation
>   [media] dvbdev: add support for media controller
>   [media] cx231xx: add media controller support
>   [media] dvb_frontend: add media controller support for DVB frontend
>   [media] dmxdev: add support for demux/dvr nodes at media controller
>   [media] dvb_ca_en50221: add support for CA node at the media
>     controller
>   [media] dvb_net: add support for DVB net node at the media controller
>   [media] dvbdev: add pad for the DVB devnodes
>   [media] tuner-core: properly initialize media controller subdev
>   [media] cx25840: fill the media controller entity
>   [media] cx231xx: initialize video/vbi pads
>   [media] cx231xx: create media links for analog mode
>   [media] dvbdev: represent frontend with two pads
>   [media] dvbdev: add a function to create DVB media graph
>   [media] cx231xx: create DVB graph
>   [media] dvbdev: enable DVB-specific links
>   [media] dvb-frontend: enable tuner link when the FE thread starts
>   [media] cx231xx: enable tuner->decoder link at videobuf start
>   [media] dvb_frontend: start media pipeline while thread is running
> 
> Zhangfei Gao (1):
>   [media] ir-hix5hd2: remove writel/readl_relaxed define
> 
>  .../DocBook/media/v4l/media-ioc-enum-entities.xml  | 102 ++++-----------
>  Documentation/DocBook/media/v4l/v4l2.xml           |   9 ++
>  drivers/media/Kconfig                              |  10 +-
>  drivers/media/dvb-core/dmxdev.c                    |  11 +-
>  drivers/media/dvb-core/dvb_ca_en50221.c            |   6 +-
>  drivers/media/dvb-core/dvb_frontend.c              | 121 ++++++++++++++++-
>  drivers/media/dvb-core/dvb_net.c                   |   6 +-
>  drivers/media/dvb-core/dvbdev.c                    | 143 ++++++++++++++++++++-
>  drivers/media/dvb-core/dvbdev.h                    |  15 +++
>  drivers/media/i2c/cx25840/cx25840-core.c           |  18 +++
>  drivers/media/i2c/cx25840/cx25840-core.h           |   3 +
>  drivers/media/rc/ir-hix5hd2.c                      |   8 --
>  drivers/media/usb/cx231xx/cx231xx-cards.c          |  98 +++++++++++++-
>  drivers/media/usb/cx231xx/cx231xx-dvb.c            |   5 +
>  drivers/media/usb/cx231xx/cx231xx-video.c          |  84 +++++++++++-
>  drivers/media/usb/cx231xx/cx231xx.h                |   5 +
>  drivers/media/v4l2-core/tuner-core.c               |  20 +++
>  drivers/media/v4l2-core/v4l2-dev.c                 |   4 +-
>  drivers/media/v4l2-core/v4l2-device.c              |   4 +-
>  include/media/media-entity.h                       |  12 +-
>  include/uapi/linux/media.h                         |  26 +++-
>  21 files changed, 592 insertions(+), 118 deletions(-)
>