[RFC PATCH v4 0/8] imx-drm dt bindings

[RFC PATCH v4 0/8] imx-drm dt bindings

[Philipp Zabel]

Hi,

here is an updated version of the imx-drm DT binding series.
These patches apply on top of Russell's imx-drm-staging branch
that just got merged int staging-next. I have added device tree
bindings between IPU and the encoders as documented in
Documentation/devicetree/bindings/media/video-interfaces.txt
and used those to determine the possible_crtcs and mux_id,
and to find all necessary components that hang off of the display
interface ports.
This allows to move the imx-drm node into the SoC level dtsi.
The existing i.MX51 and i.MX53 device trees are updated and device
tree binding documentation is included.

Changes since v3:
 - Kept the -EPROBE_DEFER in of_parse_endpoint and removed LVDS
   output ports for now. This can be revisited once drm_panel support
   is added.
 - Dropped the temporary of_graph helper copies, so this depends on
   the patch that moves the v4l2_of helpers to drivers/of.

regards
Philipp

Philipp Zabel (8):
  staging: imx-drm-core: Use OF graph to find components and connections
    between encoder and crtcs
  staging: imx-drm-core: use of_graph_parse_endpoint
  staging: imx-drm: Document updated imx-drm device tree bindings
  staging: imx-drm: Document imx-hdmi device tree bindings
  ARM: dts: imx51: Add IPU ports and endpoints, move imx-drm node to
    dtsi
  ARM: dts: imx53: Add IPU DI ports and endpoints, move imx-drm node to
    dtsi
  ARM: dts: imx6qdl: Add IPU DI ports and endpoints, move imx-drm node
    to dtsi
  staging: imx-drm: Update TODO

 .../bindings/staging/imx-drm/fsl-imx-drm.txt       |  48 ++++-
 .../devicetree/bindings/staging/imx-drm/hdmi.txt   |  53 +++++
 .../devicetree/bindings/staging/imx-drm/ldb.txt    |  20 +-
 arch/arm/boot/dts/imx51-apf51dev.dts               |  11 +-
 arch/arm/boot/dts/imx51-babbage.dts                |  28 ++-
 arch/arm/boot/dts/imx51.dtsi                       |  22 ++-
 arch/arm/boot/dts/imx53-m53evk.dts                 |  17 +-
 arch/arm/boot/dts/imx53-mba53.dts                  |  15 +-
 arch/arm/boot/dts/imx53-qsb.dts                    |  17 +-
 arch/arm/boot/dts/imx53.dtsi                       |  64 +++++-
 arch/arm/boot/dts/imx6dl.dtsi                      |  22 +--
 arch/arm/boot/dts/imx6q-sabresd.dts                |   4 -
 arch/arm/boot/dts/imx6q.dtsi                       | 124 +++++++++++-
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi             |   6 -
 arch/arm/boot/dts/imx6qdl.dtsi                     | 128 +++++++++++-
 drivers/staging/imx-drm/TODO                       |   5 -
 drivers/staging/imx-drm/imx-drm-core.c             | 215 +++++++++++++++------
 drivers/staging/imx-drm/imx-drm.h                  |   5 +-
 drivers/staging/imx-drm/imx-hdmi.c                 |   2 +-
 drivers/staging/imx-drm/imx-ldb.c                  |   4 +-
 drivers/staging/imx-drm/ipuv3-crtc.c               |  47 ++++-
 21 files changed, 703 insertions(+), 154 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt

-- 
1.8.5.3

[RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Philipp Zabel]

This patch updates the device tree binding documentation for i.MX IPU/display
nodes using the OF graph bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 .../bindings/staging/imx-drm/fsl-imx-drm.txt       | 48 +++++++++++++++++++---
 .../devicetree/bindings/staging/imx-drm/ldb.txt    | 20 +++++++--
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index b876d49..bfa19a4 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -1,3 +1,22 @@
+Freescale i.MX DRM master device
+================================
+
+The freescale i.MX DRM master device is a virtual device needed to list all
+IPU or other display interface nodes that comprise the graphics subsystem.
+
+Required properties:
+- compatible: Should be "fsl,imx-drm"
+- ports: Should contain a list of phandles pointing to display interface ports
+  of IPU devices
+
+example:
+
+imx-drm {
+	compatible = "fsl,imx-drm";
+	ports = <&ipu_di0>;
+};
+
+
 Freescale i.MX IPUv3
 ====================
 
@@ -7,18 +26,31 @@ Required properties:
   datasheet
 - interrupts: Should contain sync interrupt and error interrupt,
   in this order.
-- #crtc-cells: 1, See below
 - resets: phandle pointing to the system reset controller and
           reset line index, see reset/fsl,imx-src.txt for details
+Optional properties:
+- port@[0-3]: Port nodes with endpoint definitions as defined in
+  Documentation/devicetree/bindings/media/video-interfaces.txt.
+  Ports 0 and 1 should correspond to CSI0 and CSI1,
+  ports 2 and 3 should correspond to DI0 and DI1, respectively.
 
 example:
 
 ipu: ipu@18000000 {
-	#crtc-cells = <1>;
+	#address-cells = <1>;
+	#size-cells = <0>;
 	compatible = "fsl,imx53-ipu";
 	reg = <0x18000000 0x080000000>;
 	interrupts = <11 10>;
 	resets = <&src 2>;
+
+	ipu_di0: port@2 {
+		reg = <2>;
+
+		ipu_di0_disp0: endpoint {
+			remote-endpoint = <&display_in>;
+		};
+	};
 };
 
 Parallel display support
@@ -26,19 +58,25 @@ Parallel display support
 
 Required properties:
 - compatible: Should be "fsl,imx-parallel-display"
-- crtc: the crtc this display is connected to, see below
 Optional properties:
 - interface_pix_fmt: How this display is connected to the
-  crtc. Currently supported types: "rgb24", "rgb565", "bgr666"
+  display interface. Currently supported types: "rgb24", "rgb565", "bgr666"
 - edid: verbatim EDID data block describing attached display.
 - ddc: phandle describing the i2c bus handling the display data
   channel
+- port: A port node with endpoint definitions as defined in
+  Documentation/devicetree/bindings/media/video-interfaces.txt.
 
 example:
 
 display@di0 {
 	compatible = "fsl,imx-parallel-display";
 	edid = [edid-data];
-	crtc = <&ipu 0>;
 	interface-pix-fmt = "rgb24";
+
+	port {
+		display_in: endpoint {
+			remote-endpoint = <&ipu_di0_disp0>;
+		};
+	};
 };
diff --git a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
index ed93778..578a1fc 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
@@ -50,12 +50,14 @@ have a look at Documentation/devicetree/bindings/video/display-timing.txt.
 
 Required properties:
  - reg : should be <0> or <1>
- - crtcs : a list of phandles with index pointing to the IPU display interfaces
-           that can be used as video source for this channel.
  - fsl,data-mapping : should be "spwg" or "jeida"
                       This describes how the color bits are laid out in the
                       serialized LVDS signal.
  - fsl,data-width : should be <18> or <24>
+ - port: A port node with endpoint definitions as defined in
+   Documentation/devicetree/bindings/media/video-interfaces.txt.
+   On i.MX6, there should be four ports (port@[0-3]) that correspond
+   to the four LVDS multiplexer inputs.
 
 example:
 
@@ -77,23 +79,33 @@ ldb: ldb@53fa8008 {
 
 	lvds-channel@0 {
 		reg = <0>;
-		crtcs = <&ipu 0>;
 		fsl,data-mapping = "spwg";
 		fsl,data-width = <24>;
 
 		display-timings {
 			/* ... */
 		};
+
+		port {
+			lvds0_in: endpoint {
+				remote-endpoint = <&ipu_di0_lvds0>;
+			};
+		};
 	};
 
 	lvds-channel@1 {
 		reg = <1>;
-		crtcs = <&ipu 1>;
 		fsl,data-mapping = "spwg";
 		fsl,data-width = <24>;
 
 		display-timings {
 			/* ... */
 		};
+
+		port {
+			lvds1_in: endpoint {
+				remote-endpoint = <&ipu_di1_lvds1>;
+			};
+		};
 	};
 };
-- 
1.8.5.3

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 2609 bytes --]

On 25/02/14 16:23, Philipp Zabel wrote:

> +Freescale i.MX DRM master device
> +================================
> +
> +The freescale i.MX DRM master device is a virtual device needed to list all
> +IPU or other display interface nodes that comprise the graphics subsystem.
> +
> +Required properties:
> +- compatible: Should be "fsl,imx-drm"
> +- ports: Should contain a list of phandles pointing to display interface ports
> +  of IPU devices
> +
> +example:
> +
> +imx-drm {
> +	compatible = "fsl,imx-drm";
> +	ports = <&ipu_di0>;
> +};

I'm not a fan of having non-hardware related things in the DT data.
Especially if it makes direct references to our SW, in this case DRM.
There's no DRM on the board. I wanted to avoid all that with OMAP
display bindings.

Is there even need for such a master device? You can find all the
connected display devices from any single display device, by just
following the endpoint links.

>  display@di0 {
>  	compatible = "fsl,imx-parallel-display";
>  	edid = [edid-data];
> -	crtc = <&ipu 0>;
>  	interface-pix-fmt = "rgb24";
> +
> +	port {
> +		display_in: endpoint {
> +			remote-endpoint = <&ipu_di0_disp0>;
> +		};
> +	};
>  };

Shouldn't the pix-fmt be defined in the endpoint node? It is about pixel
format for a particular endpoint, isn't it?

> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
> index ed93778..578a1fc 100644
> --- a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
> +++ b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
> @@ -50,12 +50,14 @@ have a look at Documentation/devicetree/bindings/video/display-timing.txt.
>  
>  Required properties:
>   - reg : should be <0> or <1>
> - - crtcs : a list of phandles with index pointing to the IPU display interfaces
> -           that can be used as video source for this channel.
>   - fsl,data-mapping : should be "spwg" or "jeida"
>                        This describes how the color bits are laid out in the
>                        serialized LVDS signal.
>   - fsl,data-width : should be <18> or <24>
> + - port: A port node with endpoint definitions as defined in
> +   Documentation/devicetree/bindings/media/video-interfaces.txt.
> +   On i.MX6, there should be four ports (port@[0-3]) that correspond
> +   to the four LVDS multiplexer inputs.

Is the ldb something that's on the imx SoC?

Do you have a public branch somewhere? It'd be easier to look at the
final result, as I'm not familiar with imx.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Russell King - ARM Linux]

On Thu, Feb 27, 2014 at 01:06:52PM +0200, Tomi Valkeinen wrote:
> On 25/02/14 16:23, Philipp Zabel wrote:
> 
> > +Freescale i.MX DRM master device
> > +================================
> > +
> > +The freescale i.MX DRM master device is a virtual device needed to list all
> > +IPU or other display interface nodes that comprise the graphics subsystem.
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,imx-drm"
> > +- ports: Should contain a list of phandles pointing to display interface ports
> > +  of IPU devices
> > +
> > +example:
> > +
> > +imx-drm {
> > +	compatible = "fsl,imx-drm";
> > +	ports = <&ipu_di0>;
> > +};
> 
> I'm not a fan of having non-hardware related things in the DT data.
> Especially if it makes direct references to our SW, in this case DRM.
> There's no DRM on the board. I wanted to avoid all that with OMAP
> display bindings.
> 
> Is there even need for such a master device? You can find all the
> connected display devices from any single display device, by just
> following the endpoint links.

Please read up on what has been discussed over previous years:

http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html

and please, let's not endlessly rehash old discussions because someone
wasn't involved and has a different opinion, otherwise we're never going
to get anywhere.

Thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 649 bytes --]

On 27/02/14 13:56, Russell King - ARM Linux wrote:

>> Is there even need for such a master device? You can find all the
>> connected display devices from any single display device, by just
>> following the endpoint links.
> 
> Please read up on what has been discussed over previous years:
> 
> http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html

Thanks, that was an interesting thread. Too bad I missed it, it was
during the holiday season. And seems Laurent missed it also, as he
didn't make any replies.

The thread seemed to go over the very same things that had already been
discussed with CDF.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Russell King - ARM Linux]

On Thu, Feb 27, 2014 at 03:16:03PM +0200, Tomi Valkeinen wrote:
> On 27/02/14 13:56, Russell King - ARM Linux wrote:
> 
> >> Is there even need for such a master device? You can find all the
> >> connected display devices from any single display device, by just
> >> following the endpoint links.
> > 
> > Please read up on what has been discussed over previous years:
> > 
> > http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html
> 
> Thanks, that was an interesting thread. Too bad I missed it, it was
> during the holiday season. And seems Laurent missed it also, as he
> didn't make any replies.
> 
> The thread seemed to go over the very same things that had already been
> discussed with CDF.

That may be - but the problem with CDF solving this problem is that it's
wrong.  It's fixing what is in actual fact a *generic* problem in a much
too specific way.  To put it another way, it's forcing everyone to fix
the same problem in their own separate ways because no one is willing to
take a step back and look at the larger picture.

We can see that because ASoC has exactly the same problem - it has to
wait until all devices (DMA, CPU DAIs, codecs etc) are present before it
can initialise, just like DRM.  Can you re-use the CDF solution for ASoC?
No.  Can it be re-used elsewhere in non-display subsystems?  No.

Therefore, CDF is yet another implementation specific solution to a
generic problem which can't be re-used.

Yes, I realise that CDF may do other stuff, but because of the above, it's
a broken solution.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 2120 bytes --]

On 27/02/14 15:43, Russell King - ARM Linux wrote:

> That may be - but the problem with CDF solving this problem is that it's
> wrong.  It's fixing what is in actual fact a *generic* problem in a much
> too specific way.  To put it another way, it's forcing everyone to fix
> the same problem in their own separate ways because no one is willing to
> take a step back and look at the larger picture.
> 
> We can see that because ASoC has exactly the same problem - it has to
> wait until all devices (DMA, CPU DAIs, codecs etc) are present before it
> can initialise, just like DRM.  Can you re-use the CDF solution for ASoC?
> No.  Can it be re-used elsewhere in non-display subsystems?  No.
> 
> Therefore, CDF is yet another implementation specific solution to a
> generic problem which can't be re-used.
> 
> Yes, I realise that CDF may do other stuff, but because of the above, it's
> a broken solution.

What? Because CDF didn't fix a particular subproblem for everyone, it's
broken solution? Or did I miss your point?

The main point of CDF is not solving the initialization issue. If that
was the point, it would've been Common Initialization Framework.

The main point of CDF is to allow us to have encoder and panel drivers
that can be used by all platforms, in complex display pipeline setups.
It just also has to have some solution for the initialization problem to
get things working.

In fact, Laurent's CDF version has a solution for init problem which, I
my memory serves me right, is very much similar to yours. It just wasn't
generic. I don't remember if Laurent had a specific master node defined,
but the LCD controller was very much like it. It would be trivial to
change it to use the component helpers.

My solution is different, because I don't like the idea of requiring all
the display components to be up and running to use any of the displays.
In fact, it's not a solution at all for me, as it would prevent displays
working on boards that do not have all the display components installed,
or if the user didn't compile all the drivers.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Laurent Pinchart]

On Thursday 27 February 2014 16:10:41 Tomi Valkeinen wrote:
> On 27/02/14 15:43, Russell King - ARM Linux wrote:
> > That may be - but the problem with CDF solving this problem is that it's
> > wrong.  It's fixing what is in actual fact a *generic* problem in a much
> > too specific way.  To put it another way, it's forcing everyone to fix
> > the same problem in their own separate ways because no one is willing to
> > take a step back and look at the larger picture.
> > 
> > We can see that because ASoC has exactly the same problem - it has to
> > wait until all devices (DMA, CPU DAIs, codecs etc) are present before it
> > can initialise, just like DRM.  Can you re-use the CDF solution for ASoC?
> > No.  Can it be re-used elsewhere in non-display subsystems?  No.
> > 
> > Therefore, CDF is yet another implementation specific solution to a
> > generic problem which can't be re-used.
> > 
> > Yes, I realise that CDF may do other stuff, but because of the above, it's
> > a broken solution.
> 
> What? Because CDF didn't fix a particular subproblem for everyone, it's
> broken solution? Or did I miss your point?

Furthermore CDF was an RFC, a proof of concept implementation of the various 
components involved to solve the problems at hand. It was in no way meant to 
be merged as-is, and I would certainly have made the asynchronous registration 
code generic had I been requested to do so specifically. Unfortunately and 
sadly miscommunication lead to CDF being rejected in one block with a fuzzy 
message on how to proceed. We won't rewrite the past, but let's move forward 
in the right direction.

> The main point of CDF is not solving the initialization issue. If that
> was the point, it would've been Common Initialization Framework.
> 
> The main point of CDF is to allow us to have encoder and panel drivers
> that can be used by all platforms, in complex display pipeline setups.
> It just also has to have some solution for the initialization problem to
> get things working.
> 
> In fact, Laurent's CDF version has a solution for init problem which, I
> my memory serves me right, is very much similar to yours. It just wasn't
> generic. I don't remember if Laurent had a specific master node defined,
> but the LCD controller was very much like it. It would be trivial to
> change it to use the component helpers.

That's correct. The CDF composite device model was based on the V4L2 composite 
device model, implemented in drivers/media/v4l2-core/v4l2-async.c. Both are 
very similar in purpose to the component framework. The reason why it wasn't 
generic in the first place was that I wanted to implement a full solution as a 
proof of concept first between polishing each part independently. That turned 
out not to be the best decision ever.

> My solution is different, because I don't like the idea of requiring all
> the display components to be up and running to use any of the displays.
> In fact, it's not a solution at all for me, as it would prevent displays
> working on boards that do not have all the display components installed,
> or if the user didn't compile all the drivers.

As mentioned in my reply to Russell's component framework patch, I would like 
to base v4l2-async on top of the component framework. For this to be possible 
we need support for partial bind in the component framework, which would make 
it possible to support your use cases. Let's discuss how to achieve that in 
the other mail thread.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Philipp Zabel]

Am Donnerstag, den 27.02.2014, 13:06 +0200 schrieb Tomi Valkeinen:
> On 25/02/14 16:23, Philipp Zabel wrote:
> 
> > +Freescale i.MX DRM master device
> > +================================
> > +
> > +The freescale i.MX DRM master device is a virtual device needed to list all
> > +IPU or other display interface nodes that comprise the graphics subsystem.
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,imx-drm"
> > +- ports: Should contain a list of phandles pointing to display interface ports
> > +  of IPU devices
> > +
> > +example:
> > +
> > +imx-drm {
> > +	compatible = "fsl,imx-drm";
> > +	ports = <&ipu_di0>;
> > +};
> 
> I'm not a fan of having non-hardware related things in the DT data.
> Especially if it makes direct references to our SW, in this case DRM.
> There's no DRM on the board. I wanted to avoid all that with OMAP
> display bindings.
> 
> Is there even need for such a master device? You can find all the
> connected display devices from any single display device, by just
> following the endpoint links.

I don't particularly like this either, but it kind of has been decided.

For the i.MX6 display subsystem there is no clear single master device,
and the physical configuration changes across the SoC family. The
i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and
IPU2, with two output ports each. The i.MX6DL/i.MX6S SoCs only have one
IPU1, but it is accompanied by separate lower-power LCDIF display
controller with a single output. These may or may not be connected
indirectly across the encoder input multiplexers, so collecting them
would require scanning the whole device tree from an always-enabled
imx-drm platform device if we didn't have this node.

Also, we are free to just ignore this node in the future, if a better
way is found.

> >  display@di0 {
> >  	compatible = "fsl,imx-parallel-display";
> >  	edid = [edid-data];
> > -	crtc = <&ipu 0>;
> >  	interface-pix-fmt = "rgb24";
> > +
> > +	port {
> > +		display_in: endpoint {
> > +			remote-endpoint = <&ipu_di0_disp0>;
> > +		};
> > +	};
> >  };
> 
> Shouldn't the pix-fmt be defined in the endpoint node? It is about pixel
> format for a particular endpoint, isn't it?
> 
> > diff --git a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
> > index ed93778..578a1fc 100644
> > --- a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
> > +++ b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
> > @@ -50,12 +50,14 @@ have a look at Documentation/devicetree/bindings/video/display-timing.txt.
> >  
> >  Required properties:
> >   - reg : should be <0> or <1>
> > - - crtcs : a list of phandles with index pointing to the IPU display interfaces
> > -           that can be used as video source for this channel.
> >   - fsl,data-mapping : should be "spwg" or "jeida"
> >                        This describes how the color bits are laid out in the
> >                        serialized LVDS signal.
> >   - fsl,data-width : should be <18> or <24>
> > + - port: A port node with endpoint definitions as defined in
> > +   Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +   On i.MX6, there should be four ports (port@[0-3]) that correspond
> > +   to the four LVDS multiplexer inputs.
> 
> Is the ldb something that's on the imx SoC?

Yes. It consists of two LVDS encoders. On i.MX5 each channel is
connected to one display interface of the single IPU.
On i.MX6Q its parallel input can be connected to any of the four IPU1/2
display interfaces using a 4-port multiplexer (and on i.MX6DL it can be
connected to IPU1 or LCDIF).

> Do you have a public branch somewhere? It'd be easier to look at the
> final result, as I'm not familiar with imx.

Not yet, I will prepare a branch with the next version.

regards
Philipp

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Russell King - ARM Linux]

On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote:
> For the i.MX6 display subsystem there is no clear single master device,
> and the physical configuration changes across the SoC family. The
> i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and
> IPU2, with two output ports each.

Not also forgetting that there's another scenario too: you may wish
to drive IPU1 and IPU2 as two completely separate display subsystems
in some hardware, but as a combined display subsystem in others.

Here's another scenario.  You may have these two IPUs on the SoC, but
there's only one display output.  You want to leave the second IPU
disabled, as you wouldn't want it to be probed or even exposed to
userland.

On the face of it, the top-level super-device node doesn't look very
hardware-y, but it actually is - it's about how a board uses the
hardware provided.  This is entirely in keeping with the spirit of DT,
which is to describe what hardware is present and how it's connected
together, whether it be at the chip or board level.

If this wasn't the case, we wouldn't even attempt to describe what devices
we have on which I2C buses - we'd just list the hardware on the board
without giving any information about how it's wired together.

This is no different - however, it doesn't have (and shouldn't) be
subsystem specific... but - and this is the challenge we then face - how
do you decide that on one board with a single zImage kernel, with both
DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev
interfaces?  We could have both matching the same compatible string, but
we'd also need some way to tell each other that they're not allowed to
bind.

Before anyone argues against "it isn't hardware-y", stop and think.
What if I design a board with two Epson LCD controllers on board and
put a muxing arrangement on their output.  Is that one or two devices?
What if I want them to operate as one combined system?  What if I have
two different LCD controllers on a board.  How is this any different
from the two independent IPU hardware blocks integrated inside an iMX6
SoC with a muxing arrangement on their output?

It's very easy to look at a SoC and make the wrong decision...

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Rob Clark]

On Thu, Feb 27, 2014 at 8:00 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote:
>> For the i.MX6 display subsystem there is no clear single master device,
>> and the physical configuration changes across the SoC family. The
>> i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and
>> IPU2, with two output ports each.
>
> Not also forgetting that there's another scenario too: you may wish
> to drive IPU1 and IPU2 as two completely separate display subsystems
> in some hardware, but as a combined display subsystem in others.
>
> Here's another scenario.  You may have these two IPUs on the SoC, but
> there's only one display output.  You want to leave the second IPU
> disabled, as you wouldn't want it to be probed or even exposed to
> userland.

I agree with Russell here, purely hw description is not always going
to be enough information to know how to assemble a bag of parts into a
system.

Maybe there is some way we should be splitting this "meta" description
into different dt files or something like this (?) to make it easier
for alternative configurations, but if the hw description alone is not
enough information for the drivers to know what to do, some (for lack
of a better word) "use-case" configuration is needed, and that has to
go *somewhere*... better to put it in DT than hard code it in the
driver.

BR,
-R

> On the face of it, the top-level super-device node doesn't look very
> hardware-y, but it actually is - it's about how a board uses the
> hardware provided.  This is entirely in keeping with the spirit of DT,
> which is to describe what hardware is present and how it's connected
> together, whether it be at the chip or board level.
>
> If this wasn't the case, we wouldn't even attempt to describe what devices
> we have on which I2C buses - we'd just list the hardware on the board
> without giving any information about how it's wired together.
>
> This is no different - however, it doesn't have (and shouldn't) be
> subsystem specific... but - and this is the challenge we then face - how
> do you decide that on one board with a single zImage kernel, with both
> DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev
> interfaces?  We could have both matching the same compatible string, but
> we'd also need some way to tell each other that they're not allowed to
> bind.
>
> Before anyone argues against "it isn't hardware-y", stop and think.
> What if I design a board with two Epson LCD controllers on board and
> put a muxing arrangement on their output.  Is that one or two devices?
> What if I want them to operate as one combined system?  What if I have
> two different LCD controllers on a board.  How is this any different
> from the two independent IPU hardware blocks integrated inside an iMX6
> SoC with a muxing arrangement on their output?
>
> It's very easy to look at a SoC and make the wrong decision...
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 6092 bytes --]

On 27/02/14 15:00, Russell King - ARM Linux wrote:
> On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote:
>> For the i.MX6 display subsystem there is no clear single master device,
>> and the physical configuration changes across the SoC family. The
>> i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and
>> IPU2, with two output ports each.
> 
> Not also forgetting that there's another scenario too: you may wish
> to drive IPU1 and IPU2 as two completely separate display subsystems
> in some hardware, but as a combined display subsystem in others.
> 
> Here's another scenario.  You may have these two IPUs on the SoC, but
> there's only one display output.  You want to leave the second IPU
> disabled, as you wouldn't want it to be probed or even exposed to
> userland.

I first want to say I don't see anything wrong with such a super node.
As you say, it does describe hardware. But I also want to say that I
still don't see a need for it. Or, maybe more exactly, I don't see a
need for it in general. Maybe there are certain cases where two devices
has to be controlled by a master device. Maybe this one is one of those.

In the imx case, why wouldn't this work, without any master node, with
the IPU nodes separate in the DT data:

- One IPU enabled, one disabled: nothing special here, just set the
other IPU to status="disabled" in the DT data. The driver for the
enabled IPU would register the required DRM entities.

- Two IPUs as separate units: almost the same as above, but both would
independently register the DRM entities.

- Two IPUs in combined mode:

Pick one IPU as the master, and one as slave. Link the IPU nodes in DT
data with phandles, say: master=<&ipu1> on the slave IPU and
slave=<&ipu0> on the master.

The master one will register the DRM entities, and the slave one will
just do what the master says.

As for the probe time "are we ready yet?" problem, the IPU driver can
just delay registering the DRM entities until all the nodes in its graph
have been probed. The component helpers can probably be used here.

> On the face of it, the top-level super-device node doesn't look very
> hardware-y, but it actually is - it's about how a board uses the
> hardware provided.  This is entirely in keeping with the spirit of DT,
> which is to describe what hardware is present and how it's connected
> together, whether it be at the chip or board level.

No disagreement there. I'm mostly put off by the naming. The binding doc
says it's a "DRM master device", compatible with "fsl,imx-drm". Now,
naming may not be the most important thing in the world, but I'd rather
use generic terms, not linux driver stack names.

> If this wasn't the case, we wouldn't even attempt to describe what devices
> we have on which I2C buses - we'd just list the hardware on the board
> without giving any information about how it's wired together.
> 
> This is no different - however, it doesn't have (and shouldn't) be
> subsystem specific... but - and this is the challenge we then face - how
> do you decide that on one board with a single zImage kernel, with both
> DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev
> interfaces?  We could have both matching the same compatible string, but
> we'd also need some way to tell each other that they're not allowed to
> bind.

Yes, that's an annoying problem, we have that on OMAP. It's a clear sign
that our video support is rather messed up.

My opinion is that the fbdev and drm drivers for a single hardware
should be exclusive at compile time. We don't allow multiple drivers for
single device for other subsystems either, do we? Eventually we should
have only one driver for one hardware device.

If that's not possible, then the drivers in question could have an
option to enable or disable themselves, passed via the kernel command
line, so that the user can select which subsystem to use.

> Before anyone argues against "it isn't hardware-y", stop and think.
> What if I design a board with two Epson LCD controllers on board and
> put a muxing arrangement on their output.  Is that one or two devices?
> What if I want them to operate as one combined system?  What if I have
> two different LCD controllers on a board.  How is this any different
> from the two independent IPU hardware blocks integrated inside an iMX6
> SoC with a muxing arrangement on their output?

Well, generally speaking, I think one option is to treat the two
controllers separately and let the userspace handle it. That may or may
not be viable, depending on the hardware, but to me it resembles very
much a PC with two video cards.

If you want the two controllers to operate together more closely, you
always need special code for that particular case.

This is what CDF has been trying to accomplish: individual drivers for
each display entity, connected together via ports and endpoints. Driver
for Epson LCD controller would expose an API, that can be used handle
the LCD controller, it wouldn't make any other demands on how it's used,
is it part of DRM or fbdev, what's before or after it, etc.

Now, and I think this was your point, some kind of master device/driver
is needed to register the required DRM or fbdev entities. Usually that
can be the driver for the SoCs display controller, i.e. the first
display entity in the display pipeline. Sometimes, if it's required to
have multiple devices act together, it may be a driver specifically
designed for that purpose.

So no, I don't have a problem with master device nodes in DT. I have a
problem having pure SW stack nomenclature in the DT data (or even worse,
SW stack entities in the DT data), and I have a problem requiring
everyone to have a master device node if it's only needed for special cases.

And yes, this series is about IMX bindings, not generic ones. And I'm
also fine with requiring everyone to have a master device node, if it
can be shown that it's the only sensible approach.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Philipp Zabel]

Hi Tomi,

Am Donnerstag, den 27.02.2014, 15:55 +0200 schrieb Tomi Valkeinen:
> On 27/02/14 15:00, Russell King - ARM Linux wrote:
> > On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote:
> >> For the i.MX6 display subsystem there is no clear single master device,
> >> and the physical configuration changes across the SoC family. The
> >> i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and
> >> IPU2, with two output ports each.
> > 
> > Not also forgetting that there's another scenario too: you may wish
> > to drive IPU1 and IPU2 as two completely separate display subsystems
> > in some hardware, but as a combined display subsystem in others.
> > 
> > Here's another scenario.  You may have these two IPUs on the SoC, but
> > there's only one display output.  You want to leave the second IPU
> > disabled, as you wouldn't want it to be probed or even exposed to
> > userland.
> 
> I first want to say I don't see anything wrong with such a super node.
> As you say, it does describe hardware. But I also want to say that I
> still don't see a need for it. Or, maybe more exactly, I don't see a
> need for it in general. Maybe there are certain cases where two devices
> has to be controlled by a master device. Maybe this one is one of those.
> 
> In the imx case, why wouldn't this work, without any master node, with
> the IPU nodes separate in the DT data:
> 
> - One IPU enabled, one disabled: nothing special here, just set the
> other IPU to status="disabled" in the DT data. The driver for the
> enabled IPU would register the required DRM entities.

that should work. Let the enabled IPU create the imx-drm platform device
on probe, parse the device tree and ignore everything only hanging off
of the disabled IPU.

[Reordering a bit...]
>- Two IPUs in combined mode:
> 
> Pick one IPU as the master, and one as slave. Link the IPU nodes in DT
> data with phandles, say: master=<&ipu1> on the slave IPU and
> slave=<&ipu0> on the master.
> 
> The master one will register the DRM entities, and the slave one will
> just do what the master says.

That might work, too. Just let the each IPU scan the graph and try to
find the imx-drm master before creating the imx-drm platform device.
The first IPU fill find no preexisting master and create the imx-drm
platform device as above, adding the other IPU as well as the other
components with component_master_add_child. It just has to make sure
that the other IPU is added to the list before the encoders are.

The second IPU will scan the graph, find a preexisting master for the
other IPU node, register its component and just wait to be bound by the
master.

> - Two IPUs as separate units: almost the same as above, but both would
> independently register the DRM entities.

Here the second IPU would not be connected to the first IPU via the
graph - it would not find a preexisting imx-drm device when scanning its
graph and create its own imx-drm device just like the first IPU did.
As a result there are two completely separate DRM devices.

That being said, this change could be made at any time in the future,
in a backwards compatible fashion, by just declaring the imx-drm node
optional and ignoring it if it exists.

> As for the probe time "are we ready yet?" problem, the IPU driver can
> just delay registering the DRM entities until all the nodes in its graph
> have been probed. The component helpers can probably be used here.

This is what is happening right now, except that the two IPUs are not
obtained from the graph but are given as starting points via the ports
property in the imx-drm node.

> > On the face of it, the top-level super-device node doesn't look very
> > hardware-y, but it actually is - it's about how a board uses the
> > hardware provided.  This is entirely in keeping with the spirit of DT,
> > which is to describe what hardware is present and how it's connected
> > together, whether it be at the chip or board level.
> 
> No disagreement there. I'm mostly put off by the naming. The binding doc
> says it's a "DRM master device", compatible with "fsl,imx-drm". Now,
> naming may not be the most important thing in the world, but I'd rather
> use generic terms, not linux driver stack names.

Did anybody propose such a generic term? How about:

-imx-drm {
-	compatible = "fsl,imx-drm";
-	ports = <&ipu1_di0>, <&ipu1_di1>;
-};
+display-subsystem {
+	compatible = "fsl,imx-display-subsystem";
+	ports = <&ipu1_di0>, <&ipu1_di1>;
+};

> > If this wasn't the case, we wouldn't even attempt to describe what devices
> > we have on which I2C buses - we'd just list the hardware on the board
> > without giving any information about how it's wired together.
> > 
> > This is no different - however, it doesn't have (and shouldn't) be
> > subsystem specific... but - and this is the challenge we then face - how
> > do you decide that on one board with a single zImage kernel, with both
> > DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev
> > interfaces?  We could have both matching the same compatible string, but
> > we'd also need some way to tell each other that they're not allowed to
> > bind.
> 
> Yes, that's an annoying problem, we have that on OMAP. It's a clear sign
> that our video support is rather messed up.
> 
> My opinion is that the fbdev and drm drivers for a single hardware
> should be exclusive at compile time. We don't allow multiple drivers for
> single device for other subsystems either, do we? Eventually we should
> have only one driver for one hardware device.
> 
> If that's not possible, then the drivers in question could have an
> option to enable or disable themselves, passed via the kernel command
> line, so that the user can select which subsystem to use.

That is the exact same problem as having multiple drivers that can bind
to the same device.

> > Before anyone argues against "it isn't hardware-y", stop and think.
> > What if I design a board with two Epson LCD controllers on board and
> > put a muxing arrangement on their output.  Is that one or two devices?
> > What if I want them to operate as one combined system?  What if I have
> > two different LCD controllers on a board.  How is this any different
> > from the two independent IPU hardware blocks integrated inside an iMX6
> > SoC with a muxing arrangement on their output?
> 
> Well, generally speaking, I think one option is to treat the two
> controllers separately and let the userspace handle it. That may or may
> not be viable, depending on the hardware, but to me it resembles very
> much a PC with two video cards.

And two graphics cards connected to the same output with a multiplexer
are a mess. This only works well if it is tightly integrated.

[...]
> So no, I don't have a problem with master device nodes in DT. I have a
> problem having pure SW stack nomenclature in the DT data (or even worse,
> SW stack entities in the DT data), and I have a problem requiring
> everyone to have a master device node if it's only needed for special cases.
>
> And yes, this series is about IMX bindings, not generic ones. And I'm
> also fine with requiring everyone to have a master device node, if it
> can be shown that it's the only sensible approach.

For i.MX, for now, let's keep the mandatory imx-drm node. Maybe rename
it so nobody can say we are leaking linux subsystem names into the
device tree.

regards
Philipp

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 5666 bytes --]

On 27/02/14 18:54, Philipp Zabel wrote:

>> - One IPU enabled, one disabled: nothing special here, just set the
>> other IPU to status="disabled" in the DT data. The driver for the
>> enabled IPU would register the required DRM entities.
> 
> that should work. Let the enabled IPU create the imx-drm platform device
> on probe, parse the device tree and ignore everything only hanging off
> of the disabled IPU.

I think you misunderstood me a bit.

What I meant is that there's no need for imx-drm device at all, neither
in the DT data or in the kernel side.

There'd just be the DT nodes for the IPUs, which would cause the IPU
platform devices to be created, and a driver for the IPU. So just like
for any other normal platform device.

In the simplest cases, where only one IPU is enabled, or the IPUs want
to be considered as totally independent, there'd be nothing special. The
IPU driver would just register the drm entities.

> [Reordering a bit...]
>> - Two IPUs in combined mode:
>>
>> Pick one IPU as the master, and one as slave. Link the IPU nodes in DT
>> data with phandles, say: master=<&ipu1> on the slave IPU and
>> slave=<&ipu0> on the master.
>>
>> The master one will register the DRM entities, and the slave one will
>> just do what the master says.
> 
> That might work, too. Just let the each IPU scan the graph and try to
> find the imx-drm master before creating the imx-drm platform device.
> The first IPU fill find no preexisting master and create the imx-drm
> platform device as above, adding the other IPU as well as the other
> components with component_master_add_child. It just has to make sure
> that the other IPU is added to the list before the encoders are.
> 
> The second IPU will scan the graph, find a preexisting master for the
> other IPU node, register its component and just wait to be bound by the
> master.

Here the slave IPU doesn't need to scan the graph at all. It just needs
to make itself available somehow to the master. Maybe just by exported
functions, or registering itself somewhere.

Only the master IPU will scan the graph, and as all the entities are
connected to the same graph, including the slave IPU, the master can
find all the relevant nodes.

>> - Two IPUs as separate units: almost the same as above, but both would
>> independently register the DRM entities.
> 
> Here the second IPU would not be connected to the first IPU via the
> graph - it would not find a preexisting imx-drm device when scanning its
> graph and create its own imx-drm device just like the first IPU did.
> As a result there are two completely separate DRM devices.

I understood that that would be the idea, two separate, independent DRM
devices. Like two graphics cards on a PC.

> That being said, this change could be made at any time in the future,
> in a backwards compatible fashion, by just declaring the imx-drm node
> optional and ignoring it if it exists.

Yes, I agree.

And I don't even know if the master-slave method I described is valid,
although I don't see why it would not work. The master
"display-subsystem" DT node does make sense to me in cases like this,
where the IPUs need to be driven as a single unit.

> Did anybody propose such a generic term? How about:
> 
> -imx-drm {
> -	compatible = "fsl,imx-drm";
> -	ports = <&ipu1_di0>, <&ipu1_di1>;
> -};
> +display-subsystem {
> +	compatible = "fsl,imx-display-subsystem";
> +	ports = <&ipu1_di0>, <&ipu1_di1>;
> +};

That sounds fine to me.

I wonder how it works if, say, there are 4 IPUs, and you want to run
them in two pairs. In that case you need two of those display-subsystem
nodes. But I guess it's just a matter of assigning a number for them
with 'regs' property, and making sure the driver has nothing that
prevents multiple instances of it.

>>> If this wasn't the case, we wouldn't even attempt to describe what devices
>>> we have on which I2C buses - we'd just list the hardware on the board
>>> without giving any information about how it's wired together.
>>>
>>> This is no different - however, it doesn't have (and shouldn't) be
>>> subsystem specific... but - and this is the challenge we then face - how
>>> do you decide that on one board with a single zImage kernel, with both
>>> DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev
>>> interfaces?  We could have both matching the same compatible string, but
>>> we'd also need some way to tell each other that they're not allowed to
>>> bind.
>>
>> Yes, that's an annoying problem, we have that on OMAP. It's a clear sign
>> that our video support is rather messed up.
>>
>> My opinion is that the fbdev and drm drivers for a single hardware
>> should be exclusive at compile time. We don't allow multiple drivers for
>> single device for other subsystems either, do we? Eventually we should
>> have only one driver for one hardware device.
>>
>> If that's not possible, then the drivers in question could have an
>> option to enable or disable themselves, passed via the kernel command
>> line, so that the user can select which subsystem to use.
> 
> That is the exact same problem as having multiple drivers that can bind
> to the same device.

Hmm, sorry? Weren't we just discussing about that problem =). Or maybe I
missed the original point from Russell, as the problem about DRM and
fbdev conflicting sounded a bit unrelated.

> For i.MX, for now, let's keep the mandatory imx-drm node. Maybe rename
> it so nobody can say we are leaking linux subsystem names into the
> device tree.

Sounds good to me.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings

[Philipp Zabel]

Hi Tomi,

Am Freitag, den 28.02.2014, 09:36 +0200 schrieb Tomi Valkeinen:
> On 27/02/14 18:54, Philipp Zabel wrote:
> 
> >> - One IPU enabled, one disabled: nothing special here, just set the
> >> other IPU to status="disabled" in the DT data. The driver for the
> >> enabled IPU would register the required DRM entities.
> > 
> > that should work. Let the enabled IPU create the imx-drm platform device
> > on probe, parse the device tree and ignore everything only hanging off
> > of the disabled IPU.
> 
> I think you misunderstood me a bit.
> 
> What I meant is that there's no need for imx-drm device at all, neither
> in the DT data or in the kernel side.

agreed about the DT node. But the drm_device must be associated with a
struct device. There is no reason to have the device representing the
whole display subsystem hang off of one IPU over the other, and I'd
rather not special case the IPU code with master/slave cases.

> There'd just be the DT nodes for the IPUs, which would cause the IPU
> platform devices to be created, and a driver for the IPU. So just like
> for any other normal platform device.

Except that one IPU has to create the drm_device and the other needs to
be drm_device-less.

> In the simplest cases, where only one IPU is enabled, or the IPUs want
> to be considered as totally independent, there'd be nothing special. The
> IPU driver would just register the drm entities.

[...]
> Here the slave IPU doesn't need to scan the graph at all. It just needs
> to make itself available somehow to the master. Maybe just by exported
> functions, or registering itself somewhere.
>
> Only the master IPU will scan the graph, and as all the entities are
> connected to the same graph, including the slave IPU, the master can
> find all the relevant nodes.

Somehow it needs to know whether it should create the component master
and drm_device, or just register as a slave component. I'd strongly
prefer to have a look at the graph over giving explicit "master;" or
"slave;" properties to the IPU device nodes for this single
Linux-specific reason.

[...]
> And I don't even know if the master-slave method I described is valid,
> although I don't see why it would not work. The master
> "display-subsystem" DT node does make sense to me in cases like this,
> where the IPUs need to be driven as a single unit.

which is a sensible default for i.MX6.

> > Did anybody propose such a generic term? How about:
> > 
> > -imx-drm {
> > -	compatible = "fsl,imx-drm";
> > -	ports = <&ipu1_di0>, <&ipu1_di1>;
> > -};
> > +display-subsystem {
> > +	compatible = "fsl,imx-display-subsystem";
> > +	ports = <&ipu1_di0>, <&ipu1_di1>;
> > +};
> 
> That sounds fine to me.
> 
> I wonder how it works if, say, there are 4 IPUs, and you want to run
> them in two pairs. In that case you need two of those display-subsystem
> nodes. But I guess it's just a matter of assigning a number for them
> with 'regs' property, and making sure the driver has nothing that
> prevents multiple instances of it.

Exactly. I'm not even sure it is necessary to assign a number.

[...]
> >> If that's not possible, then the drivers in question could have an
> >> option to enable or disable themselves, passed via the kernel command
> >> line, so that the user can select which subsystem to use.
> > 
> > That is the exact same problem as having multiple drivers that can bind
> > to the same device.
> 
> Hmm, sorry? Weren't we just discussing about that problem =). Or maybe I
> missed the original point from Russell, as the problem about DRM and
> fbdev conflicting sounded a bit unrelated.

I guess I missed it, though. Rereading the context, I am just
(re)stating that this is a problem beyond DRM and fbdev. Compiling a
kernel with two drivers binding to the same device, you effectively say
"whichever binds first, I don't care". To change this, we'd need
something similar to module blacklists for non-modular builds. I'd
rather strive to remove the need for duplicate drivers in the kernel.

regards
Philipp

[RFC PATCH v4 4/8] staging: imx-drm: Document imx-hdmi device tree bindings

[Philipp Zabel]

This patch adds device tree binding documentation for the HDMI transmitter
on i.MX6.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 .../devicetree/bindings/staging/imx-drm/hdmi.txt   | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt

diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
new file mode 100644
index 0000000..7dcd673a
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
@@ -0,0 +1,53 @@
+Device-Tree bindings for HDMI Transmitter
+
+HDMI Transmitter
+================
+
+The LVDS Display Bridge device tree node contains up to two lvds-channel
+nodes describing each of the two LVDS encoder channels of the bridge.
+
+Required properties:
+ - #address-cells : should be <1>
+ - #size-cells : should be <0>
+ - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
+ - gpr : should be <&gpr>.
+   The phandle points to the iomuxc-gpr region containing the HDMI
+   multiplexer control register.
+ - clocks, clock-names : phandles to the HDMI iahb and isrf clocks, as described
+   in Documentation/devicetree/bindings/clock/clock-bindings.txt and
+   Documentation/devicetree/bindings/clock/imx6q-clock.txt.
+ - port@[0-4]: Up to four port nodes with endpoint definitions as defined in
+   Documentation/devicetree/bindings/media/video-interfaces.txt,
+   corresponding to the four inputs to the HDMI multiplexer.
+
+example:
+
+	gpr: iomuxc-gpr@020e0000 {
+		/* ... */
+	};
+
+        hdmi: hdmi@0120000 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <0x00120000 0x9000>;
+                interrupts = <0 115 0x04>;
+                gpr = <&gpr>;
+                clocks = <&clks 123>, <&clks 124>;
+                clock-names = "iahb", "isfr";
+
+                port@0 {
+                        reg = <0>;
+
+                        hdmi_mux_0: endpoint {
+                                remote-endpoint = <&ipu1_di0_hdmi>;
+                        };
+                };
+
+                port@1 {
+                        reg = <1>;
+
+                        hdmi_mux_1: endpoint {
+                                remote-endpoint = <&ipu1_di1_hdmi>;
+                        };
+                };
+        };
-- 
1.8.5.3

Re: [RFC PATCH v4 4/8] staging: imx-drm: Document imx-hdmi device tree bindings

[Fabio Estevam]

On Tue, Feb 25, 2014 at 11:23 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> This patch adds device tree binding documentation for the HDMI transmitter
> on i.MX6.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  .../devicetree/bindings/staging/imx-drm/hdmi.txt   | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>
> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> new file mode 100644
> index 0000000..7dcd673a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> @@ -0,0 +1,53 @@
> +Device-Tree bindings for HDMI Transmitter
> +
> +HDMI Transmitter
> +================
> +
> +The LVDS Display Bridge device tree node contains up to two lvds-channel
> +nodes describing each of the two LVDS encoder channels of the bridge.

Copy and paste error? ;-)

Re: [RFC PATCH v4 4/8] staging: imx-drm: Document imx-hdmi device tree bindings

[Philipp Zabel]

Hi Fabio,

Am Dienstag, den 25.02.2014, 12:13 -0300 schrieb Fabio Estevam:
> On Tue, Feb 25, 2014 at 11:23 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > This patch adds device tree binding documentation for the HDMI transmitter
> > on i.MX6.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  .../devicetree/bindings/staging/imx-drm/hdmi.txt   | 53 ++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >
> > diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> > new file mode 100644
> > index 0000000..7dcd673a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> > @@ -0,0 +1,53 @@
> > +Device-Tree bindings for HDMI Transmitter
> > +
> > +HDMI Transmitter
> > +================
> > +
> > +The LVDS Display Bridge device tree node contains up to two lvds-channel
> > +nodes describing each of the two LVDS encoder channels of the bridge.
> 
> Copy and paste error? ;-)

Thanks, yes. I don't have anything interesting to say about the HDMI TX
there, so I'll just drop this sentence.

regards
Philipp

Re: [RFC PATCH v4 4/8] staging: imx-drm: Document imx-hdmi device tree bindings

[Fabio Estevam]

On Tue, Feb 25, 2014 at 11:23 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> +Required properties:
> + - #address-cells : should be <1>
> + - #size-cells : should be <0>
> + - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
> + - gpr : should be <&gpr>.
> +   The phandle points to the iomuxc-gpr region containing the HDMI
> +   multiplexer control register.
> + - clocks, clock-names : phandles to the HDMI iahb and isrf clocks, as described
> +   in Documentation/devicetree/bindings/clock/clock-bindings.txt and
> +   Documentation/devicetree/bindings/clock/imx6q-clock.txt.
> + - port@[0-4]: Up to four port nodes with endpoint definitions as defined in
> +   Documentation/devicetree/bindings/media/video-interfaces.txt,
> +   corresponding to the four inputs to the HDMI multiplexer.
> +
> +example:
> +
> +       gpr: iomuxc-gpr@020e0000 {
> +               /* ... */
> +       };
> +
> +        hdmi: hdmi@0120000 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <0x00120000 0x9000>;
> +                interrupts = <0 115 0x04>;
> +                gpr = <&gpr>;
> +                clocks = <&clks 123>, <&clks 124>;
> +                clock-names = "iahb", "isfr";

The 'compatible' property is missing in the example.

[RFC PATCH v4 1/8] staging: imx-drm-core: Use OF graph to find components and connections between encoder and crtcs

[Philipp Zabel]

From: Philipp Zabel <philipp.zabel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch adds support to find the involved components connected to
the IPU display interface ports using the OF graph bindings documented
in Documentation/devicetree/bindings/media/video-interfaces.txt.
It makes use of the of_graph (formerly v4l2_of) parsing helpers and
thus depends on the patch that moves those out to drivers/of.

Each display interface needs to have an associated port node in the
device tree. We can associate this node with the crtc platform device
and use it to find the crtc corresponding to a given port node instead
of using a combination of parent device node and id number, as before.

Explicitly converting the void* cookie to the port device tree node
allows to get rid of the ipu_id and di_id fields. The multiplexer
setting on i.MX6 now can be obtained from the port id (reg property)
in the device tree.

The imx-drm node now needs a ports property that contains phandles
to each of the IPU display interface port nodes. From there, all
attached encoders are scanned and enabled encoders are added to a
waiting list.
The bind order makes sure that once all components are probed, crtcs
are bound before encoders, so that imx_drm_encoder_parse_of can be
called from the encoder bind callbacks.

For parsing the OF graph, temporary copies of the V4L2 OF graph
helpers are used, that can be removed again once those are available
at a generic place.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v3:
 - Retain -EPROBE_DEFER behavior in imx_drm_encoder_parse_of
 - Use of_graph helpers in drivers/of
---
 drivers/staging/imx-drm/imx-drm-core.c | 215 +++++++++++++++++++++++----------
 drivers/staging/imx-drm/imx-drm.h      |   5 +-
 drivers/staging/imx-drm/imx-hdmi.c     |   2 +-
 drivers/staging/imx-drm/imx-ldb.c      |   4 +-
 drivers/staging/imx-drm/ipuv3-crtc.c   |  47 +++++--
 5 files changed, 198 insertions(+), 75 deletions(-)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index dcba518..01bc1cf 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -17,6 +17,7 @@
 #include <linux/device.h>
 #include <linux/fb.h>
 #include <linux/module.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <drm/drmP.h>
 #include <drm/drm_fb_helper.h>
@@ -30,6 +31,11 @@
 
 struct imx_drm_crtc;
 
+struct imx_drm_component {
+	struct device_node *of_node;
+	struct list_head list;
+};
+
 struct imx_drm_device {
 	struct drm_device			*drm;
 	struct imx_drm_crtc			*crtc[MAX_CRTC];
@@ -41,9 +47,7 @@ struct imx_drm_crtc {
 	struct drm_crtc				*crtc;
 	int					pipe;
 	struct imx_drm_crtc_helper_funcs	imx_drm_helper_funcs;
-	void					*cookie;
-	int					id;
-	int					mux_id;
+	struct device_node			*port;
 };
 
 static int legacyfb_depth = 16;
@@ -341,14 +345,11 @@ err_kms:
 
 /*
  * imx_drm_add_crtc - add a new crtc
- *
- * The return value if !NULL is a cookie for the caller to pass to
- * imx_drm_remove_crtc later.
  */
 int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
 		struct imx_drm_crtc **new_crtc,
 		const struct imx_drm_crtc_helper_funcs *imx_drm_helper_funcs,
-		void *cookie, int id)
+		struct device_node *port)
 {
 	struct imx_drm_device *imxdrm = drm->dev_private;
 	struct imx_drm_crtc *imx_drm_crtc;
@@ -370,9 +371,7 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
 
 	imx_drm_crtc->imx_drm_helper_funcs = *imx_drm_helper_funcs;
 	imx_drm_crtc->pipe = imxdrm->pipes++;
-	imx_drm_crtc->cookie = cookie;
-	imx_drm_crtc->id = id;
-	imx_drm_crtc->mux_id = imx_drm_crtc->pipe;
+	imx_drm_crtc->port = port;
 	imx_drm_crtc->crtc = crtc;
 
 	imxdrm->crtc[imx_drm_crtc->pipe] = imx_drm_crtc;
@@ -416,49 +415,56 @@ int imx_drm_remove_crtc(struct imx_drm_crtc *imx_drm_crtc)
 EXPORT_SYMBOL_GPL(imx_drm_remove_crtc);
 
 /*
- * Find the DRM CRTC possible mask for the device node cookie/id.
+ * Find the DRM CRTC possible mask for the connected endpoint.
  *
  * The encoder possible masks are defined by their position in the
  * mode_config crtc_list.  This means that CRTCs must not be added
  * or removed once the DRM device has been fully initialised.
  */
 static uint32_t imx_drm_find_crtc_mask(struct imx_drm_device *imxdrm,
-	void *cookie, int id)
+	struct device_node *endpoint)
 {
+	struct device_node *port;
 	unsigned i;
 
+	port = of_graph_get_remote_port(endpoint);
+	if (!port)
+		return 0;
+	of_node_put(port);
+
 	for (i = 0; i < MAX_CRTC; i++) {
 		struct imx_drm_crtc *imx_drm_crtc = imxdrm->crtc[i];
-		if (imx_drm_crtc && imx_drm_crtc->id == id &&
-		    imx_drm_crtc->cookie == cookie)
+		if (imx_drm_crtc && imx_drm_crtc->port == port)
 			return drm_crtc_mask(imx_drm_crtc->crtc);
 	}
 
 	return 0;
 }
 
+static struct device_node *imx_drm_of_get_next_endpoint(
+		const struct device_node *parent, struct device_node *prev)
+{
+	struct device_node *node = of_graph_get_next_endpoint(parent, prev);
+	of_node_put(prev);
+	return node;
+}
+
 int imx_drm_encoder_parse_of(struct drm_device *drm,
 	struct drm_encoder *encoder, struct device_node *np)
 {
 	struct imx_drm_device *imxdrm = drm->dev_private;
+	struct device_node *ep = NULL;
 	uint32_t crtc_mask = 0;
-	int i, ret = 0;
+	int i;
 
-	for (i = 0; !ret; i++) {
-		struct of_phandle_args args;
-		uint32_t mask;
-		int id;
+	for (i = 0; ; i++) {
+		u32 mask;
 
-		ret = of_parse_phandle_with_args(np, "crtcs", "#crtc-cells", i,
-						 &args);
-		if (ret == -ENOENT)
+		ep = imx_drm_of_get_next_endpoint(np, ep);
+		if (!ep)
 			break;
-		if (ret < 0)
-			return ret;
 
-		id = args.args_count > 0 ? args.args[0] : 0;
-		mask = imx_drm_find_crtc_mask(imxdrm, args.np, id);
-		of_node_put(args.np);
+		mask = imx_drm_find_crtc_mask(imxdrm, ep);
 
 		/*
 		 * If we failed to find the CRTC(s) which this encoder is
@@ -472,6 +478,11 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
 		crtc_mask |= mask;
 	}
 
+	if (ep)
+		of_node_put(ep);
+	if (i == 0)
+		return -ENOENT;
+
 	encoder->possible_crtcs = crtc_mask;
 
 	/* FIXME: this is the mask of outputs which can clone this output. */
@@ -481,11 +492,36 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
 }
 EXPORT_SYMBOL_GPL(imx_drm_encoder_parse_of);
 
-int imx_drm_encoder_get_mux_id(struct drm_encoder *encoder)
+/*
+ * @node: device tree node containing encoder input ports
+ * @encoder: drm_encoder
+ */
+int imx_drm_encoder_get_mux_id(struct device_node *node,
+			       struct drm_encoder *encoder)
 {
 	struct imx_drm_crtc *imx_crtc = imx_drm_find_crtc(encoder->crtc);
+	struct device_node *ep = NULL;
+	struct device_node *port;
+	int id, ret;
+
+	if (!node || !imx_crtc)
+		return -EINVAL;
+
+	do {
+		ep = imx_drm_of_get_next_endpoint(node, ep);
+		if (!ep)
+			break;
+
+		port = of_graph_get_remote_port(ep);
+		of_node_put(port);
+		if (port == imx_crtc->port) {
+			ret = of_property_read_u32(ep->parent, "reg", &id);
+			of_node_put(ep);
+			return ret ? ret : id;
+		}
+	} while (ep);
 
-	return imx_crtc ? imx_crtc->mux_id : -EINVAL;
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(imx_drm_encoder_get_mux_id);
 
@@ -528,48 +564,29 @@ static struct drm_driver imx_drm_driver = {
 	.patchlevel		= 0,
 };
 
-static int compare_parent_of(struct device *dev, void *data)
-{
-	struct of_phandle_args *args = data;
-	return dev->parent && dev->parent->of_node == args->np;
-}
-
 static int compare_of(struct device *dev, void *data)
 {
-	return dev->of_node == data;
-}
-
-static int imx_drm_add_components(struct device *master, struct master *m)
-{
-	struct device_node *np = master->of_node;
-	unsigned i;
-	int ret;
+	struct device_node *np = data;
 
-	for (i = 0; ; i++) {
-		struct of_phandle_args args;
-
-		ret = of_parse_phandle_with_fixed_args(np, "crtcs", 1,
-						       i, &args);
-		if (ret)
-			break;
-
-		ret = component_master_add_child(m, compare_parent_of, &args);
-		of_node_put(args.np);
-
-		if (ret)
-			return ret;
+	/* Special case for LDB, one device for two channels */
+	if (of_node_cmp(np->name, "lvds-channel") == 0) {
+		np = of_get_parent(np);
+		of_node_put(np);
 	}
 
-	for (i = 0; ; i++) {
-		struct device_node *node;
+	return dev->of_node == np;
+}
 
-		node = of_parse_phandle(np, "connectors", i);
-		if (!node)
-			break;
+static LIST_HEAD(imx_drm_components);
 
-		ret = component_master_add_child(m, compare_of, node);
-		of_node_put(node);
+static int imx_drm_add_components(struct device *master, struct master *m)
+{
+	struct imx_drm_component *component;
+	int ret;
 
+	list_for_each_entry(component, &imx_drm_components, list) {
+		ret = component_master_add_child(m, compare_of,
+						 component->of_node);
 		if (ret)
 			return ret;
 	}
@@ -592,9 +609,81 @@ static const struct component_master_ops imx_drm_ops = {
 	.unbind = imx_drm_unbind,
 };
 
+static struct imx_drm_component *imx_drm_find_component(struct device *dev,
+		struct device_node *node)
+{
+	struct imx_drm_component *component;
+
+	list_for_each_entry(component, &imx_drm_components, list)
+		if (component->of_node == node)
+			return component;
+
+	return NULL;
+}
+
+static int imx_drm_add_component(struct device *dev, struct device_node *node)
+{
+	struct imx_drm_component *component;
+
+	if (imx_drm_find_component(dev, node))
+		return 0;
+
+	component = devm_kzalloc(dev, sizeof(*component), GFP_KERNEL);
+	if (!component)
+		return -ENOMEM;
+
+	component->of_node = node;
+	list_add_tail(&component->list, &imx_drm_components);
+
+	return 0;
+}
+
 static int imx_drm_platform_probe(struct platform_device *pdev)
 {
+	struct device_node *ep, *port, *remote;
 	int ret;
+	int i;
+
+	/*
+	 * Bind the IPU display interface ports first, so that
+	 * imx_drm_encoder_parse_of called from encoder .bind callbacks
+	 * works as expected.
+	 */
+	for (i = 0; ; i++) {
+		port = of_parse_phandle(pdev->dev.of_node, "ports", i);
+		if (!port)
+			break;
+
+		ret = imx_drm_add_component(&pdev->dev, port);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (i == 0) {
+		dev_err(&pdev->dev, "missing 'ports' property\n");
+		return -ENODEV;
+	}
+
+	/* Then bind all encoders */
+	for (i = 0; ; i++) {
+		port = of_parse_phandle(pdev->dev.of_node, "ports", i);
+		if (!port)
+			break;
+
+		for_each_child_of_node(port, ep) {
+			remote = of_graph_get_remote_port_parent(ep);
+			if (!remote || !of_device_is_available(remote)) {
+				of_node_put(remote);
+				continue;
+			}
+
+			ret = imx_drm_add_component(&pdev->dev, remote);
+			of_node_put(remote);
+			if (ret < 0)
+				return ret;
+		}
+		of_node_put(port);
+	}
 
 	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
 	if (ret)
diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h
index 035ab62..a322bac 100644
--- a/drivers/staging/imx-drm/imx-drm.h
+++ b/drivers/staging/imx-drm/imx-drm.h
@@ -26,7 +26,7 @@ struct imx_drm_crtc_helper_funcs {
 int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
 		struct imx_drm_crtc **new_crtc,
 		const struct imx_drm_crtc_helper_funcs *imx_helper_funcs,
-		void *cookie, int id);
+		struct device_node *port);
 int imx_drm_remove_crtc(struct imx_drm_crtc *);
 int imx_drm_init_drm(struct platform_device *pdev,
 		int preferred_bpp);
@@ -45,7 +45,8 @@ int imx_drm_panel_format_pins(struct drm_encoder *encoder,
 int imx_drm_panel_format(struct drm_encoder *encoder,
 		u32 interface_pix_fmt);
 
-int imx_drm_encoder_get_mux_id(struct drm_encoder *encoder);
+int imx_drm_encoder_get_mux_id(struct device_node *node,
+		struct drm_encoder *encoder);
 int imx_drm_encoder_parse_of(struct drm_device *drm,
 	struct drm_encoder *encoder, struct device_node *np);
 
diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c
index 87268b4..7807e80 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -1455,7 +1455,7 @@ static void imx_hdmi_encoder_prepare(struct drm_encoder *encoder)
 static void imx_hdmi_encoder_commit(struct drm_encoder *encoder)
 {
 	struct imx_hdmi *hdmi = container_of(encoder, struct imx_hdmi, encoder);
-	int mux = imx_drm_encoder_get_mux_id(encoder);
+	int mux = imx_drm_encoder_get_mux_id(hdmi->dev->of_node, encoder);
 
 	imx_hdmi_set_ipu_di_mux(hdmi, mux);
 
diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
index 5168c76..301c430 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -168,7 +168,7 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
 	u32 pixel_fmt;
 	unsigned long serial_clk;
 	unsigned long di_clk = mode->clock * 1000;
-	int mux = imx_drm_encoder_get_mux_id(encoder);
+	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->child, encoder);
 
 	if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) {
 		/* dual channel LVDS mode */
@@ -203,7 +203,7 @@ static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
 	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
 	struct imx_ldb *ldb = imx_ldb_ch->ldb;
 	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
-	int mux = imx_drm_encoder_get_mux_id(encoder);
+	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->child, encoder);
 
 	if (dual) {
 		clk_prepare_enable(ldb->clk[0]);
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 3e0854a..59c79b2 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -350,10 +350,8 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 		return ret;
 	}
 
-	ret = imx_drm_add_crtc(drm, &ipu_crtc->base,
-			&ipu_crtc->imx_crtc,
-			&ipu_crtc_helper_funcs,
-			ipu_crtc->dev->parent->of_node, pdata->di);
+	ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc,
+			&ipu_crtc_helper_funcs, ipu_crtc->dev->of_node);
 	if (ret) {
 		dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
 		goto err_put_resources;
@@ -401,6 +399,28 @@ err_put_resources:
 	return ret;
 }
 
+static struct device_node *ipu_drm_get_port_by_id(struct device_node *parent,
+						  int port_id)
+{
+	struct device_node *port;
+	int id, ret;
+
+	port = of_get_child_by_name(parent, "port");
+	while (port) {
+		ret = of_property_read_u32(port, "reg", &id);
+		if (!ret && id == port_id)
+			return port;
+
+		do {
+			port = of_get_next_child(parent, port);
+			if (!port)
+				return NULL;
+		} while (of_node_cmp(port->name, "port"));
+	}
+
+	return NULL;
+}
+
 static int ipu_drm_bind(struct device *dev, struct device *master, void *data)
 {
 	struct ipu_client_platformdata *pdata = dev->platform_data;
@@ -441,16 +461,29 @@ static const struct component_ops ipu_crtc_ops = {
 
 static int ipu_drm_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	struct ipu_client_platformdata *pdata = dev->platform_data;
 	int ret;
 
-	if (!pdev->dev.platform_data)
+	if (!dev->platform_data)
 		return -EINVAL;
 
-	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+	if (!dev->of_node) {
+		/* Associate crtc device with the corresponding DI port node */
+		dev->of_node = ipu_drm_get_port_by_id(dev->parent->of_node,
+						      pdata->di + 2);
+		if (!dev->of_node) {
+			dev_err(dev, "missing port@%d node in %s\n",
+				pdata->di + 2, dev->parent->of_node->full_name);
+			return -ENODEV;
+		}
+	}
+
+	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	if (ret)
 		return ret;
 
-	return component_add(&pdev->dev, &ipu_crtc_ops);
+	return component_add(dev, &ipu_crtc_ops);
 }
 
 static int ipu_drm_remove(struct platform_device *pdev)
-- 
1.8.5.3

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

[RFC PATCH v4 2/8] staging: imx-drm-core: use of_graph_parse_endpoint

[Philipp Zabel]

Using of_graph_parse_endpoint recovers the port id from an endpoint device
tree node. This just replaces an open coded read of the "reg" property.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/staging/imx-drm/imx-drm-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index 01bc1cf..009805a 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -501,8 +501,9 @@ int imx_drm_encoder_get_mux_id(struct device_node *node,
 {
 	struct imx_drm_crtc *imx_crtc = imx_drm_find_crtc(encoder->crtc);
 	struct device_node *ep = NULL;
+	struct of_endpoint endpoint;
 	struct device_node *port;
-	int id, ret;
+	int ret;
 
 	if (!node || !imx_crtc)
 		return -EINVAL;
@@ -515,9 +516,8 @@ int imx_drm_encoder_get_mux_id(struct device_node *node,
 		port = of_graph_get_remote_port(ep);
 		of_node_put(port);
 		if (port == imx_crtc->port) {
-			ret = of_property_read_u32(ep->parent, "reg", &id);
-			of_node_put(ep);
-			return ret ? ret : id;
+			ret = of_graph_parse_endpoint(ep, &endpoint);
+			return ret ? ret : endpoint.id;
 		}
 	} while (ep);
 
-- 
1.8.5.3

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

[RFC PATCH v4 5/8] ARM: dts: imx51: Add IPU ports and endpoints, move imx-drm node to dtsi

[Philipp Zabel]

This patch connects IPU and and parallel display device tree
nodes using the OF graph bindings described in
Documentation/devicetree/bindings/media/video-interfaces.txt

The IPU ports correspond to the two display interfaces. The
order of endpoints in the ports is arbitrary.

Since the imx-drm node now only needs to contain links to the
display interfaces, it can be moved to the SoC dtsi level. At
the board level, only connections between the display interface
ports and panels have to be added.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx51-apf51dev.dts | 11 ++++++++++-
 arch/arm/boot/dts/imx51-babbage.dts  | 28 ++++++++++++++++++++--------
 arch/arm/boot/dts/imx51.dtsi         | 22 +++++++++++++++++++++-
 3 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts b/arch/arm/boot/dts/imx51-apf51dev.dts
index 5a7f552..d3f9814 100644
--- a/arch/arm/boot/dts/imx51-apf51dev.dts
+++ b/arch/arm/boot/dts/imx51-apf51dev.dts
@@ -18,7 +18,6 @@
 
 	display@di1 {
 		compatible = "fsl,imx-parallel-display";
-		crtcs = <&ipu 0>;
 		interface-pix-fmt = "bgr666";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_ipu_disp1_1>;
@@ -41,6 +40,12 @@
 				pixelclk-active = <0>;
 			};
 		};
+
+		port {
+			display_in: endpoint {
+				remote-endpoint = <&ipu_di0_disp0>;
+			};
+		};
 	};
 
 	gpio-keys {
@@ -122,3 +127,7 @@
 		};
 	};
 };
+
+&ipu_di0_disp0 {
+	remote-endpoint = <&display_in>;
+};
diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index 6ff15a0..6719271 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -23,7 +23,6 @@
 
 	display0: display@di0 {
 		compatible = "fsl,imx-parallel-display";
-		crtcs = <&ipu 0>;
 		interface-pix-fmt = "rgb24";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_ipu_disp1_1>;
@@ -41,11 +40,16 @@
 				vsync-len = <10>;
 			};
 		};
+
+		port {
+			display0_in: endpoint {
+				remote-endpoint = <&ipu_di0_disp0>;
+			};
+		};
 	};
 
 	display1: display@di1 {
 		compatible = "fsl,imx-parallel-display";
-		crtcs = <&ipu 1>;
 		interface-pix-fmt = "rgb565";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_ipu_disp2_1>;
@@ -68,6 +72,12 @@
 				pixelclk-active = <0>;
 			};
 		};
+
+		port {
+			display1_in: endpoint {
+				remote-endpoint = <&ipu_di1_disp1>;
+			};
+		};
 	};
 
 	gpio-keys {
@@ -81,12 +91,6 @@
 		};
 	};
 
-	imx-drm {
-		compatible = "fsl,imx-drm";
-		crtcs = <&ipu 0>, <&ipu 1>;
-		connectors = <&display0>, <&display1>;
-	};
-
 	sound {
 		compatible = "fsl,imx51-babbage-sgtl5000",
 			     "fsl,imx-audio-sgtl5000";
@@ -264,6 +268,14 @@
 	};
 };
 
+&ipu_di0_disp0 {
+	remote-endpoint = <&display0_in>;
+};
+
+&ipu_di1_disp1 {
+	remote-endpoint = <&display1_in>;
+};
+
 &ssi2 {
 	fsl,mode = "i2s-slave";
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 4bcdd3a..536644c 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -79,6 +79,11 @@
 		};
 	};
 
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		ports = <&ipu_di0>, <&ipu_di1>;
+	};
+
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
@@ -92,13 +97,28 @@
 		};
 
 		ipu: ipu@40000000 {
-			#crtc-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 			compatible = "fsl,imx51-ipu";
 			reg = <0x40000000 0x20000000>;
 			interrupts = <11 10>;
 			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
 			clock-names = "bus", "di0", "di1";
 			resets = <&src 2>;
+
+			ipu_di0: port@2 {
+				reg = <2>;
+
+				ipu_di0_disp0: endpoint {
+				};
+			};
+
+			ipu_di1: port@3 {
+				reg = <3>;
+
+				ipu_di1_disp1: endpoint {
+				};
+			};
 		};
 
 		aips@70000000 { /* AIPS1 */
-- 
1.8.5.3

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

[RFC PATCH v4 7/8] ARM: dts: imx6qdl: Add IPU DI ports and endpoints, move imx-drm node to dtsi

[Philipp Zabel]

This patch connects IPU and display encoder (HDMI, LVDS, MIPI)
device tree nodes, as well as parallel displays on the DISP0
and DISP1 outputs, using the OF graph bindings described in
Documentation/devicetree/bindings/media/video-interfaces.txt

The IPU ports correspond to the two display interfaces. The
order of endpoints in the ports is arbitrary.

Each encoder with an associated input multiplexer has multiple
input ports in the device tree. The order and reg property of
the ports must correspond to the multiplexer input order.

Since the imx-drm node now only needs to contain links to the
display interfaces, it can be moved to the SoC dtsi level. At
the board level, only connections between the display interface
ports and encoders or panels have to be added.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v3:
 - Removed port@4 output nodes from lvds-channel nodes for now.
---
 arch/arm/boot/dts/imx6dl.dtsi          |  22 +++---
 arch/arm/boot/dts/imx6q-sabresd.dts    |   4 --
 arch/arm/boot/dts/imx6q.dtsi           | 124 ++++++++++++++++++++++++++++++--
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi |   6 --
 arch/arm/boot/dts/imx6qdl.dtsi         | 128 ++++++++++++++++++++++++++++++++-
 5 files changed, 253 insertions(+), 31 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 6dc3970..8ba94b8 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -70,6 +70,15 @@
 			};
 		};
 	};
+
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		ports = <&ipu1_di0>, <&ipu1_di1>;
+	};
+};
+
+&hdmi {
+	compatible = "fsl,imx6dl-hdmi";
 };
 
 &ldb {
@@ -79,17 +88,4 @@
 	clock-names = "di0_pll", "di1_pll",
 		      "di0_sel", "di1_sel",
 		      "di0", "di1";
-
-	lvds-channel@0 {
-		crtcs = <&ipu1 0>, <&ipu1 1>;
-	};
-
-	lvds-channel@1 {
-		crtcs = <&ipu1 0>, <&ipu1 1>;
-	};
-};
-
-&hdmi {
-	compatible = "fsl,imx6dl-hdmi";
-	crtcs = <&ipu1 0>, <&ipu1 1>;
 };
diff --git a/arch/arm/boot/dts/imx6q-sabresd.dts b/arch/arm/boot/dts/imx6q-sabresd.dts
index 66f220a..9cbdfe7 100644
--- a/arch/arm/boot/dts/imx6q-sabresd.dts
+++ b/arch/arm/boot/dts/imx6q-sabresd.dts
@@ -20,10 +20,6 @@
 	compatible = "fsl,imx6q-sabresd", "fsl,imx6q";
 };
 
-&imx_drm {
-	crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
-};
-
 &sata {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 187fe33..db356e6 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -132,13 +132,84 @@
 		};
 
 		ipu2: ipu@02800000 {
-			#crtc-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 			compatible = "fsl,imx6q-ipu";
 			reg = <0x02800000 0x400000>;
 			interrupts = <0 8 0x4 0 7 0x4>;
 			clocks = <&clks 133>, <&clks 134>, <&clks 137>;
 			clock-names = "bus", "di0", "di1";
 			resets = <&src 4>;
+
+			ipu2_di0: port@2 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <2>;
+
+				ipu2_di0_disp0: endpoint@0 {
+				};
+
+				ipu2_di0_hdmi: endpoint@1 {
+					remote-endpoint = <&hdmi_mux_2>;
+				};
+
+				ipu2_di0_mipi: endpoint@2 {
+				};
+
+				ipu2_di0_lvds0: endpoint@3 {
+					remote-endpoint = <&lvds0_mux_2>;
+				};
+
+				ipu2_di0_lvds1: endpoint@4 {
+					remote-endpoint = <&lvds1_mux_2>;
+				};
+			};
+
+			ipu2_di1: port@3 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <3>;
+
+				ipu2_di1_hdmi: endpoint@1 {
+					remote-endpoint = <&hdmi_mux_3>;
+				};
+
+				ipu2_di1_mipi: endpoint@2 {
+				};
+
+				ipu2_di1_lvds0: endpoint@3 {
+					remote-endpoint = <&lvds0_mux_3>;
+				};
+
+				ipu2_di1_lvds1: endpoint@4 {
+					remote-endpoint = <&lvds1_mux_3>;
+				};
+			};
+		};
+	};
+
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		ports = <&ipu1_di0>, <&ipu1_di1>, <&ipu2_di0>, <&ipu2_di1>;
+	};
+};
+
+&hdmi {
+	compatible = "fsl,imx6q-hdmi";
+
+	port@2 {
+		reg = <2>;
+
+		hdmi_mux_2: endpoint {
+			remote-endpoint = <&ipu2_di0_hdmi>;
+		};
+	};
+
+	port@3 {
+		reg = <3>;
+
+		hdmi_mux_3: endpoint {
+			remote-endpoint = <&ipu2_di1_hdmi>;
 		};
 	};
 };
@@ -152,15 +223,56 @@
 		      "di0", "di1";
 
 	lvds-channel@0 {
-		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+		port@2 {
+			reg = <2>;
+
+			lvds0_mux_2: endpoint {
+				remote-endpoint = <&ipu2_di0_lvds0>;
+			};
+		};
+
+		port@3 {
+			reg = <3>;
+
+			lvds0_mux_3: endpoint {
+				remote-endpoint = <&ipu2_di1_lvds0>;
+			};
+		};
 	};
 
 	lvds-channel@1 {
-		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+		port@2 {
+			reg = <2>;
+
+			lvds1_mux_2: endpoint {
+				remote-endpoint = <&ipu2_di0_lvds1>;
+			};
+		};
+
+		port@3 {
+			reg = <3>;
+
+			lvds1_mux_3: endpoint {
+				remote-endpoint = <&ipu2_di1_lvds1>;
+			};
+		};
 	};
 };
 
-&hdmi {
-	compatible = "fsl,imx6q-hdmi";
-	crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+&mipi_dsi {
+	port@2 {
+		reg = <2>;
+
+		mipi_mux_2: endpoint {
+			remote-endpoint = <&ipu2_di0_mipi>;
+		};
+	};
+
+	port@3 {
+		reg = <3>;
+
+		mipi_mux_3: endpoint {
+			remote-endpoint = <&ipu2_di1_mipi>;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index dfca3e0..e75e11b 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -62,12 +62,6 @@
 		};
 	};
 
-	imx_drm: imx-drm {
-		compatible = "fsl,imx-drm";
-		crtcs = <&ipu1 0>, <&ipu1 1>;
-		connectors = <&ldb>;
-	};
-
 	sound {
 		compatible = "fsl,imx6q-sabresd-wm8962",
 			   "fsl,imx-audio-wm8962";
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 930ebe0..64a8cbe 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1358,23 +1358,77 @@
 				status = "disabled";
 
 				lvds-channel@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
 					reg = <0>;
 					status = "disabled";
+
+					port@0 {
+						reg = <0>;
+
+						lvds0_mux_0: endpoint {
+							remote-endpoint = <&ipu1_di0_lvds0>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						lvds0_mux_1: endpoint {
+							remote-endpoint = <&ipu1_di1_lvds0>;
+						};
+					};
 				};
 
 				lvds-channel@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
 					reg = <1>;
 					status = "disabled";
+
+					port@0 {
+						reg = <0>;
+
+						lvds1_mux_0: endpoint {
+							remote-endpoint = <&ipu1_di0_lvds1>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						lvds1_mux_1: endpoint {
+							remote-endpoint = <&ipu1_di1_lvds1>;
+						};
+					};
 				};
 			};
 
 			hdmi: hdmi@0120000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x00120000 0x9000>;
 				interrupts = <0 115 0x04>;
 				gpr = <&gpr>;
 				clocks = <&clks 123>, <&clks 124>;
 				clock-names = "iahb", "isfr";
 				status = "disabled";
+
+				port@0 {
+					reg = <0>;
+
+					hdmi_mux_0: endpoint {
+						remote-endpoint = <&ipu1_di0_hdmi>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					hdmi_mux_1: endpoint {
+						remote-endpoint = <&ipu1_di1_hdmi>;
+					};
+				};
 			};
 
 			dcic1: dcic@020e4000 {
@@ -1588,8 +1642,27 @@
 				reg = <0x021dc000 0x4000>;
 			};
 
-			mipi@021e0000 { /* MIPI-DSI */
+			mipi_dsi: mipi@021e0000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x021e0000 0x4000>;
+				status = "disabled";
+
+				port@0 {
+					reg = <0>;
+
+					mipi_mux_0: endpoint {
+						remote-endpoint = <&ipu1_di0_mipi>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					mipi_mux_1: endpoint {
+						remote-endpoint = <&ipu1_di1_mipi>;
+					};
+				};
 			};
 
 			vdoa@021e4000 {
@@ -1643,13 +1716,64 @@
 		};
 
 		ipu1: ipu@02400000 {
-			#crtc-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 			compatible = "fsl,imx6q-ipu";
 			reg = <0x02400000 0x400000>;
 			interrupts = <0 6 0x4 0 5 0x4>;
 			clocks = <&clks 130>, <&clks 131>, <&clks 132>;
 			clock-names = "bus", "di0", "di1";
 			resets = <&src 2>;
+
+			ipu1_di0: port@2 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <2>;
+
+				ipu1_di0_disp0: endpoint@0 {
+				};
+
+				ipu1_di0_hdmi: endpoint@1 {
+					remote-endpoint = <&hdmi_mux_0>;
+				};
+
+				ipu1_di0_mipi: endpoint@2 {
+					remote-endpoint = <&mipi_mux_0>;
+				};
+
+				ipu1_di0_lvds0: endpoint@3 {
+					remote-endpoint = <&lvds0_mux_0>;
+				};
+
+				ipu1_di0_lvds1: endpoint@4 {
+					remote-endpoint = <&lvds1_mux_0>;
+				};
+			};
+
+			ipu1_di1: port@3 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <3>;
+
+				ipu1_di0_disp1: endpoint@0 {
+				};
+
+				ipu1_di1_hdmi: endpoint@1 {
+					remote-endpoint = <&hdmi_mux_1>;
+				};
+
+				ipu1_di1_mipi: endpoint@2 {
+					remote-endpoint = <&mipi_mux_1>;
+				};
+
+				ipu1_di1_lvds0: endpoint@3 {
+					remote-endpoint = <&lvds0_mux_1>;
+				};
+
+				ipu1_di1_lvds1: endpoint@4 {
+					remote-endpoint = <&lvds1_mux_1>;
+				};
+			};
 		};
 	};
 };
-- 
1.8.5.3

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

[RFC PATCH v4 6/8] ARM: dts: imx53: Add IPU DI ports and endpoints, move imx-drm node to dtsi

[Philipp Zabel]

This patch connects IPU and display encoder (VGA, LVDS)
device tree nodes, as well as parallel displays on the DISP0
and DISP1 outputs, using the OF graph bindings described in
Documentation/devicetree/bindings/media/video-interfaces.txt

The IPU ports correspond to the two display interfaces. The
order of endpoints in the ports is arbitrary.

Since the imx-drm node now only needs to contain links to the
display interfaces, it can be moved to the SoC dtsi level. At
the board level, only connections between the display interface
ports and encoders or panels have to be added.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 arch/arm/boot/dts/imx53-m53evk.dts | 17 +++++-----
 arch/arm/boot/dts/imx53-mba53.dts  | 15 +++++----
 arch/arm/boot/dts/imx53-qsb.dts    | 17 +++++-----
 arch/arm/boot/dts/imx53.dtsi       | 64 +++++++++++++++++++++++++++++++++++---
 4 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index ee6107b..0298adc 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -23,7 +23,6 @@
 	soc {
 		display1: display@di1 {
 			compatible = "fsl,imx-parallel-display";
-			crtcs = <&ipu 1>;
 			interface-pix-fmt = "bgr666";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_ipu_disp2_1>;
@@ -44,6 +43,12 @@
 				};
 			};
 		};
+
+		port {
+			display1_in: endpoint {
+				remote-endpoint = <&ipu_di1_disp1>;
+			};
+		};
 	};
 
 	backlight {
@@ -53,12 +58,6 @@
 		default-brightness-level = <6>;
 	};
 
-	imx-drm {
-		compatible = "fsl,imx-drm";
-		crtcs = <&ipu 1>;
-		connectors = <&display1>;
-	};
-
 	leds {
 		compatible = "gpio-leds";
 		pinctrl-names = "default";
@@ -227,6 +226,10 @@
 	};
 };
 
+&ipu_di1_disp1 {
+	remote-endpoint = <&display1_in>;
+};
+
 &nfc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_nand_1>;
diff --git a/arch/arm/boot/dts/imx53-mba53.dts b/arch/arm/boot/dts/imx53-mba53.dts
index 9b6e769..3c3d69e 100644
--- a/arch/arm/boot/dts/imx53-mba53.dts
+++ b/arch/arm/boot/dts/imx53-mba53.dts
@@ -38,15 +38,14 @@
 		compatible = "fsl,imx-parallel-display";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_disp1_1>;
-		crtcs = <&ipu 1>;
 		interface-pix-fmt = "rgb24";
 		status = "disabled";
-	};
 
-	imx-drm {
-		compatible = "fsl,imx-drm";
-		crtcs = <&ipu 1>;
-		connectors = <&disp1>, <&tve>;
+		port {
+			display1_in: endpoint {
+				remote-endpoint = <&ipu_di1_disp1>;
+			};
+		};
 	};
 
 	reg_3p2v: 3p2v {
@@ -147,6 +146,10 @@
 	};
 };
 
+&ipu_di1_disp1 {
+	remote-endpoint = <&display1_in>;
+};
+
 &cspi {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx53-qsb.dts b/arch/arm/boot/dts/imx53-qsb.dts
index 3cb4f77..8b25428 100644
--- a/arch/arm/boot/dts/imx53-qsb.dts
+++ b/arch/arm/boot/dts/imx53-qsb.dts
@@ -23,7 +23,6 @@
 
 	display0: display@di0 {
 		compatible = "fsl,imx-parallel-display";
-		crtcs = <&ipu 0>;
 		interface-pix-fmt = "rgb565";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_ipu_disp0_1>;
@@ -46,6 +45,12 @@
 				pixelclk-active = <0>;
 			};
 		};
+
+		port {
+			display0_in: endpoint {
+				remote-endpoint = <&ipu_di0_disp0>;
+			};
+		};
 	};
 
 	gpio-keys {
@@ -72,12 +77,6 @@
 		};
 	};
 
-	imx-drm {
-		compatible = "fsl,imx-drm";
-		crtcs = <&ipu 0>;
-		connectors = <&display0>;
-	};
-
 	leds {
 		compatible = "gpio-leds";
 		pinctrl-names = "default";
@@ -132,6 +131,10 @@
 	status = "okay";
 };
 
+&ipu_di0_disp0 {
+	remote-endpoint = <&display0_in>;
+};
+
 &ssi2 {
 	fsl,mode = "i2s-slave";
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 4307e80..5b89b91 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -45,6 +45,11 @@
 		};
 	};
 
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		ports = <&ipu_di0>, <&ipu_di1>;
+	};
+
 	tzic: tz-interrupt-controller@0fffc000 {
 		compatible = "fsl,imx53-tzic", "fsl,tzic";
 		interrupt-controller;
@@ -85,13 +90,49 @@
 		ranges;
 
 		ipu: ipu@18000000 {
-			#crtc-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 			compatible = "fsl,imx53-ipu";
 			reg = <0x18000000 0x080000000>;
 			interrupts = <11 10>;
 			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
 			clock-names = "bus", "di0", "di1";
 			resets = <&src 2>;
+
+			ipu_di0: port@2 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <2>;
+
+				ipu_di0_disp0: endpoint@0 {
+					reg = <0>;
+				};
+
+				ipu_di0_lvds0: endpoint@1 {
+					reg = <1>;
+					remote-endpoint = <&lvds0_in>;
+				};
+			};
+
+			ipu_di1: port@3 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <3>;
+
+				ipu_di1_disp1: endpoint@0 {
+					reg = <0>;
+				};
+
+				ipu_di1_lvds1: endpoint@1 {
+					reg = <1>;
+					remote-endpoint = <&lvds1_in>;
+				};
+
+				ipu_di1_tve: endpoint@2 {
+					reg = <2>;
+					remote-endpoint = <&tve_in>;
+				};
+			};
 		};
 
 		aips@50000000 { /* AIPS1 */
@@ -838,14 +879,24 @@
 
 				lvds-channel@0 {
 					reg = <0>;
-					crtcs = <&ipu 0>;
 					status = "disabled";
+
+					port {
+						lvds0_in: endpoint {
+							remote-endpoint = <&ipu_di0_lvds0>;
+						};
+					};
 				};
 
 				lvds-channel@1 {
 					reg = <1>;
-					crtcs = <&ipu 1>;
 					status = "disabled";
+
+					port {
+						lvds1_in: endpoint {
+							remote-endpoint = <&ipu_di0_lvds0>;
+						};
+					};
 				};
 			};
 
@@ -1103,8 +1154,13 @@
 				interrupts = <92>;
 				clocks = <&clks 69>, <&clks 116>;
 				clock-names = "tve", "di_sel";
-				crtcs = <&ipu 1>;
 				status = "disabled";
+
+				port {
+					tve_in: endpoint {
+						remote-endpoint = <&ipu_di1_tve>;
+					};
+				};
 			};
 
 			vpu: vpu@63ff4000 {
-- 
1.8.5.3

[RFC PATCH v4 8/8] staging: imx-drm: Update TODO

[Philipp Zabel]

The device tree bindings are updated regardless of the common display
framework and in the meantime the HDMI driver was included.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/TODO | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/imx-drm/TODO b/drivers/staging/imx-drm/TODO
index 6a9da94..29636fb 100644
--- a/drivers/staging/imx-drm/TODO
+++ b/drivers/staging/imx-drm/TODO
@@ -1,15 +1,10 @@
 TODO:
 - get DRM Maintainer review for this code
-- Wait for common display framework to hit mainline and update the IPU
-  driver to use it. This will most probably make changes to the devicetree
-  bindings necessary.
-- Factor out more code to common helper functions
 - decide where to put the base driver. It is not specific to a subsystem
   and would be used by DRM/KMS and media/V4L2
 
 Missing features (not necessarily for moving out of staging):
 
-- Add i.MX6 HDMI support
 - Add support for IC (Image converter)
 - Add support for CSI (CMOS Sensor interface)
 - Add support for VDIC (Video Deinterlacer)
-- 
1.8.5.3