diff mbox

[v7,4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI

Message ID cd1caff75b6069af98680f78d672546748cb43c5.1480414715.git.moinejf@free.fr (mailing list archive)
State New, archived
Headers show

Commit Message

Jean-Francois Moine Nov. 29, 2016, 9:08 a.m. UTC
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 .../devicetree/bindings/display/sunxi/hdmi.txt     | 56 ++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt

Comments

Laurent Pinchart Nov. 29, 2016, 6:46 p.m. UTC | #1
Hi Jean-François,

Thank you for the patch.

On Tuesday 29 Nov 2016 10:08:25 Jean-Francois Moine wrote:
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  .../devicetree/bindings/display/sunxi/hdmi.txt     | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt new file mode
> 100644
> index 0000000..1e107cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> @@ -0,0 +1,56 @@
> +Allwinner HDMI Transmitter
> +==========================
> +
> +The Allwinner HDMI transmitters are included in the SoCs.
> +They support audio and video.
> +
> +Required properties:
> + - compatible : should be one of
> +		"allwinner,sun8i-a83t-hdmi"
> +		"allwinner,sun8i-h3-hdmi"
> + - reg: base address and size of the I/O memory
> + - clocks : phandles to the HDMI clocks as described in
> +	Documentation/devicetree/bindings/clock/clock-bindings.txt
> + - clock-names : must be
> +		"bus" : bus gate
> +		"clock" : streaming clock
> +		"ddc-clock" : DDC clock
> + - resets : One or two phandles to the HDMI resets
> + - reset-names : when 2 phandles, must be
> +		"hdmi0" and "hdmi1"
> + - #address-cells : should be <1>
> + - #size-cells : should be <0>
> +
> +Required nodes:
> + - port: Audio and video input port nodes with endpoint definitions
> +	as defined in Documentation/devicetree/bindings/graph.txt.
> +	port@0 is video and port@1 is audio.
> +
> +Example:
> +
> +	hdmi: hdmi@01ee0000 {
> +		compatible = "allwinner,sun8i-a83t-hdmi";
> +		reg = <0x01ee0000 0x20000>;
> +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> +			 <&ccu CLK_HDMI_DDC>;
> +		clock-names = "bus", "clock", "ddc-clock";
> +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> +		reset-names = "hdmi0", "hdmi1";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hdmi_pins_a>;
> +		status = "disabled";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@0 {			/* video */
> +			reg = <0>;
> +			hdmi_tcon1: endpoint {
> +				remote-endpoint = <&tcon1_hdmi>;
> +			};
> +		};
> +		port@1 {			/* audio */
> +			reg = <1>;
> +			hdmi_i2s2: endpoint {
> +				remote-endpoint = <&i2s2_hdmi>;
> +			};
> +		};

You need a third port for the HDMI encoder output, connected to an HDMI 
connector DT node.

> +	};
Jean-Francois Moine Nov. 29, 2016, 7:27 p.m. UTC | #2
On Tue, 29 Nov 2016 20:46:22 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
	[snip]
> > +Example:
> > +
> > +	hdmi: hdmi@01ee0000 {
> > +		compatible = "allwinner,sun8i-a83t-hdmi";
> > +		reg = <0x01ee0000 0x20000>;
> > +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> > +			 <&ccu CLK_HDMI_DDC>;
> > +		clock-names = "bus", "clock", "ddc-clock";
> > +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> > +		reset-names = "hdmi0", "hdmi1";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&hdmi_pins_a>;
> > +		status = "disabled";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		port@0 {			/* video */
> > +			reg = <0>;
> > +			hdmi_tcon1: endpoint {
> > +				remote-endpoint = <&tcon1_hdmi>;
> > +			};
> > +		};
> > +		port@1 {			/* audio */
> > +			reg = <1>;
> > +			hdmi_i2s2: endpoint {
> > +				remote-endpoint = <&i2s2_hdmi>;
> > +			};
> > +		};
> 
> You need a third port for the HDMI encoder output, connected to an HDMI 
> connector DT node.

I don't see what you mean. The HDMI device is both the encoder
and connector (as the TDA998x):

plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
----- plane ------    - CRTC -   - encoder  \
                                   connector -- (HDMI cable)
         audio-controller -   - audio-codec /

> > +	};
Laurent Pinchart Nov. 29, 2016, 7:33 p.m. UTC | #3
Hi Jean-François,

On Tuesday 29 Nov 2016 20:27:51 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 20:46:22 +0200 Laurent Pinchart wrote:
> [snip]
> 
> >> +Example:
> >> +
> >> +	hdmi: hdmi@01ee0000 {
> >> +		compatible = "allwinner,sun8i-a83t-hdmi";
> >> +		reg = <0x01ee0000 0x20000>;
> >> +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> >> +			 <&ccu CLK_HDMI_DDC>;
> >> +		clock-names = "bus", "clock", "ddc-clock";
> >> +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> >> +		reset-names = "hdmi0", "hdmi1";
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&hdmi_pins_a>;
> >> +		status = "disabled";
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +		port@0 {			/* video */
> >> +			reg = <0>;
> >> +			hdmi_tcon1: endpoint {
> >> +				remote-endpoint = <&tcon1_hdmi>;
> >> +			};
> >> +		};
> >> +		port@1 {			/* audio */
> >> +			reg = <1>;
> >> +			hdmi_i2s2: endpoint {
> >> +				remote-endpoint = <&i2s2_hdmi>;
> >> +			};
> >> +		};
> > 
> > You need a third port for the HDMI encoder output, connected to an HDMI
> > connector DT node.
> 
> I don't see what you mean. The HDMI device is both the encoder
> and connector (as the TDA998x):

The driver might create both an encoder and a connector, but I very much doubt 
that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector, unless the 
SoC package has an HDMI connector coming out of it :-)

> plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> ----- plane ------    - CRTC -   - encoder  \
>                                    connector -- (HDMI cable)
>          audio-controller -   - audio-codec /
> 
> > > +	};
Jean-Francois Moine Nov. 29, 2016, 8:04 p.m. UTC | #4
On Tue, 29 Nov 2016 21:33 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> > > You need a third port for the HDMI encoder output, connected to an HDMI
> > > connector DT node.
> > 
> > I don't see what you mean. The HDMI device is both the encoder
> > and connector (as the TDA998x):
> 
> The driver might create both an encoder and a connector, but I very much doubt 
> that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector, unless the 
> SoC package has an HDMI connector coming out of it :-)
> 
> > plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> > ----- plane ------    - CRTC -   - encoder  \
> >                                    connector -- (HDMI cable)
> >          audio-controller -   - audio-codec /

The schema is the same as the Dove Cubox: the TDA998x is just a chip
with some wires going out and the physical connector is supposed to be
at the end of the wires.

Here, the HDMI pins of the SoC go to a pure hardware chip and then to
the physical connector. Which software entity do you want to add?
Laurent Pinchart Nov. 29, 2016, 8:10 p.m. UTC | #5
Hi Jean-François,

On Tuesday 29 Nov 2016 21:04:55 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 21:33 +0200 Laurent Pinchart wrote:
> >>> You need a third port for the HDMI encoder output, connected to an
> >>> HDMI connector DT node.
> >> 
> >> I don't see what you mean. The HDMI device is both the encoder
> >> and connector (as the TDA998x):
> >
> > The driver might create both an encoder and a connector, but I very much
> > doubt that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector,
> > unless the SoC package has an HDMI connector coming out of it :-)
> > 
> >> plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> >> ----- plane ------    - CRTC -   - encoder  \
> >>                                    connector -- (HDMI cable)
> >>          audio-controller -   - audio-codec /
> 
> The schema is the same as the Dove Cubox: the TDA998x is just a chip
> with some wires going out and the physical connector is supposed to be
> at the end of the wires.

I've missed the Dove Cubox DT bindings when they were submitted. Fortunately 
(or unfortunately for you, depending on how you look at it ;-)) I've paid more 
attention this time.

> Here, the HDMI pins of the SoC go to a pure hardware chip and then to
> the physical connector. Which software entity do you want to add?

I don't want to add a software entity, I just want to model the connector in 
DT as it's present in the system. Even though that's more common for other bus 
types than HDMI (LVDS for instance) it wouldn't be inconceivable to connect 
the HDMI signals to an on-board chim instead of an HDMI connector, so the HDMI 
encoder output should be modelled by a port and connected to a connector DT 
node in this case.
Jean-Francois Moine Nov. 30, 2016, 8:12 a.m. UTC | #6
On Tue, 29 Nov 2016 22:10:01 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Jean-François,
> 
> On Tuesday 29 Nov 2016 21:04:55 Jean-Francois Moine wrote:
> > On Tue, 29 Nov 2016 21:33 +0200 Laurent Pinchart wrote:
> > >>> You need a third port for the HDMI encoder output, connected to an
> > >>> HDMI connector DT node.
> > >> 
> > >> I don't see what you mean. The HDMI device is both the encoder
> > >> and connector (as the TDA998x):
> > >
> > > The driver might create both an encoder and a connector, but I very much
> > > doubt that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector,
> > > unless the SoC package has an HDMI connector coming out of it :-)
> > > 
> > >> plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> > >> ----- plane ------    - CRTC -   - encoder  \
> > >>                                    connector -- (HDMI cable)
> > >>          audio-controller -   - audio-codec /
> > 
> > The schema is the same as the Dove Cubox: the TDA998x is just a chip
> > with some wires going out and the physical connector is supposed to be
> > at the end of the wires.
> 
> I've missed the Dove Cubox DT bindings when they were submitted. Fortunately 
> (or unfortunately for you, depending on how you look at it ;-)) I've paid more 
> attention this time.
> 
> > Here, the HDMI pins of the SoC go to a pure hardware chip and then to
> > the physical connector. Which software entity do you want to add?
> 
> I don't want to add a software entity, I just want to model the connector in 
> DT as it's present in the system. Even though that's more common for other bus 
> types than HDMI (LVDS for instance) it wouldn't be inconceivable to connect 
> the HDMI signals to an on-board chim instead of an HDMI connector, so the HDMI 
> encoder output should be modelled by a port and connected to a connector DT 
> node in this case.

Well, I don't see what this connector can be.
May you give me a DT example?
Laurent Pinchart Nov. 30, 2016, 8:20 a.m. UTC | #7
Hi Jean-François,

On Wednesday 30 Nov 2016 09:12:08 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 22:10:01 +0200 Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 21:04:55 Jean-Francois Moine wrote:
> >> On Tue, 29 Nov 2016 21:33 +0200 Laurent Pinchart wrote:
> >>>>> You need a third port for the HDMI encoder output, connected to an
> >>>>> HDMI connector DT node.
> >>>> 
> >>>> I don't see what you mean. The HDMI device is both the encoder
> >>> 
> >>>> and connector (as the TDA998x):
> >>> The driver might create both an encoder and a connector, but I very
> >>> much doubt that the "allwinner,sun8i-a83t-hdmi" hardware contains a
> >>> connector, unless the SoC package has an HDMI connector coming out of
> >>> it :-)
> >>> 
> >>>> plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> >>>> ----- plane ------    - CRTC -   - encoder  \
> >>>>                                    connector -- (HDMI cable)
> >>>>          audio-controller -   - audio-codec /
> >> 
> >> The schema is the same as the Dove Cubox: the TDA998x is just a chip
> >> with some wires going out and the physical connector is supposed to be
> >> at the end of the wires.
> > 
> > I've missed the Dove Cubox DT bindings when they were submitted.
> > Fortunately (or unfortunately for you, depending on how you look at it
> > ;-)) I've paid more attention this time.
> > 
> >> Here, the HDMI pins of the SoC go to a pure hardware chip and then to
> >> the physical connector. Which software entity do you want to add?
> > 
> > I don't want to add a software entity, I just want to model the connector
> > in DT as it's present in the system. Even though that's more common for
> > other bus types than HDMI (LVDS for instance) it wouldn't be
> > inconceivable to connect the HDMI signals to an on-board chim instead of
> > an HDMI connector, so the HDMI encoder output should be modelled by a
> > port and connected to a connector DT node in this case.
> 
> Well, I don't see what this connector can be.
> May you give me a DT example?

Sure.

arch/arm/boot/dts/r8a7791-koelsch.dts

        /* HDMI encoder */

        hdmi@39 {
                compatible = "adi,adv7511w";
                reg = <0x39>;
                interrupt-parent = <&gpio3>;
                interrupts = <29 IRQ_TYPE_LEVEL_LOW>;

                adi,input-depth = <8>;
                adi,input-colorspace = "rgb";
                adi,input-clock = "1x";
                adi,input-style = <1>;
                adi,input-justification = "evenly";

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        port@0 {
                                reg = <0>;
                                adv7511_in: endpoint {
                                        remote-endpoint = <&du_out_rgb>;
                                };
                        };

                        port@1 {
                                reg = <1>;
                                adv7511_out: endpoint {
                                        remote-endpoint = <&hdmi_con>;
                                };
                        };
                };
        };

        /* HDMI connector */

        hdmi-out {
                compatible = "hdmi-connector";
                type = "a";

                port {
                        hdmi_con: endpoint {
                                remote-endpoint = <&adv7511_out>;
                        };
                };
        };
Jean-Francois Moine Nov. 30, 2016, 9:27 a.m. UTC | #8
On Wed, 30 Nov 2016 10:20:21 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> > Well, I don't see what this connector can be.
> > May you give me a DT example?
> 
> Sure.
> 
> arch/arm/boot/dts/r8a7791-koelsch.dts
> 
>         /* HDMI encoder */
> 
>         hdmi@39 {
>                 compatible = "adi,adv7511w";
>                 reg = <0x39>;
>                 interrupt-parent = <&gpio3>;
>                 interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> 
>                 adi,input-depth = <8>;
>                 adi,input-colorspace = "rgb";
>                 adi,input-clock = "1x";
>                 adi,input-style = <1>;
>                 adi,input-justification = "evenly";
> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         port@0 {
>                                 reg = <0>;
>                                 adv7511_in: endpoint {
>                                         remote-endpoint = <&du_out_rgb>;
>                                 };
>                         };
> 
>                         port@1 {
>                                 reg = <1>;
>                                 adv7511_out: endpoint {
>                                         remote-endpoint = <&hdmi_con>;
>                                 };
>                         };
>                 };
>         };
> 
>         /* HDMI connector */
> 
>         hdmi-out {
>                 compatible = "hdmi-connector";
>                 type = "a";
> 
>                 port {
>                         hdmi_con: endpoint {
>                                 remote-endpoint = <&adv7511_out>;
>                         };
>                 };
>         };

Hi Laurent,

Sorry for I don't see the interest:
- it is obvious that the HDMI connector is a 'hdmi-connector'!
- the physical connector type may be changed on any board by a soldering
  iron or a connector to connector cable.
- what does the software do with the connector type?
- why not to put the connector information in the HDMI device?

And, if I follow you, the graph of ports could also be used to describe
the way the various parts of the SoCs are powered, to describe the pin
connections, to describe the USB connectors, to describe the board
internal hubs and bridges...
Laurent Pinchart Nov. 30, 2016, 9:52 a.m. UTC | #9
Hi Jean-François,

On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
> On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
> >> Well, I don't see what this connector can be.
> >> May you give me a DT example?
> > 
> > Sure.
> > 
> > arch/arm/boot/dts/r8a7791-koelsch.dts
> > 
> >         /* HDMI encoder */
> >         
> >         hdmi@39 {
> >                 compatible = "adi,adv7511w";
> >                 reg = <0x39>;
> >                 interrupt-parent = <&gpio3>;
> >                 interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> >                 
> >                 adi,input-depth = <8>;
> >                 adi,input-colorspace = "rgb";
> >                 adi,input-clock = "1x";
> >                 adi,input-style = <1>;
> >                 adi,input-justification = "evenly";
> >                 
> >                 ports {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         
> >                         port@0 {
> >                                 reg = <0>;
> >                                 adv7511_in: endpoint {
> >                                         remote-endpoint = <&du_out_rgb>;
> >                                 };
> >                         };
> >                         
> >                         port@1 {
> >                                 reg = <1>;
> >                                 adv7511_out: endpoint {
> >                                         remote-endpoint = <&hdmi_con>;
> >                                 };
> >                         };
> >                 };
> >         
> >         };
> >         
> >         /* HDMI connector */
> >         
> >         hdmi-out {
> >                 compatible = "hdmi-connector";
> >                 type = "a";
> >                 
> >                 port {
> >                         hdmi_con: endpoint {
> >                                 remote-endpoint = <&adv7511_out>;
> >                         };
> >                 };
> >         };
> 
> Hi Laurent,
> 
> Sorry for I don't see the interest:
> - it is obvious that the HDMI connector is a 'hdmi-connector'!

It still has to be told to the drivers, they don't know how to identify a 
connector by looking at it :-)

> - the physical connector type may be changed on any board by a soldering
>   iron or a connector to connector cable.

Which is also true for any other component on the board. DT (and for that 
matter any firmware description of the platform) isn't soldering-proof.

> - what does the software do with the connector type?

That's up to the software to decide, the DT bindings should describe the 
hardware in the most accurate and usable way for the OS as possible. One of my 
longer term goals is to add connector drivers to handle DDC and HPD when 
they're not handled by the encoder (they are in the above example).

If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD 
to a GPIO, we would have

	hdmi-out {
		compatible = "hdmi-connector";
		type = "a";
		/* I2C bus and GPIO references are made up for the example */
		ddc-i2c-bus = <&i2c4>;
		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>

		port {
			hdmi_con: endpoint {
				remote-endpoint = <&adv7511_out>;
			};
		};
	};

and both HPD and EDID reading should be handled by the connector driver.

> - why not to put the connector information in the HDMI device?

Because the connector is separate from the encoder. It's not uncommon 
(depending on the encoder type) to have the encoder output connected to a non-
connector entity such as another chained encoder.

For example most LVDS encoders are connected to a panel, but I have a board 
with the following encoders chain.

CRTC -- parallel RGB --> on-SoC LVDS encoder -- LVDS --> on-board LVDS decoder 
-- parallel RGB --> HDMI encoder -- HDMI --> HDMI connector

I can't support that if the LVDS encoder driver hardcodes the assumption that 
the encoder output is connected to a panel. This kind of usage might be less 
common for HDMI but is certainly not inconceivable.

> And, if I follow you, the graph of ports could also be used to describe
> the way the various parts of the SoCs are powered, to describe the pin
> connections, to describe the USB connectors, to describe the board
> internal hubs and bridges...

It should be used where applicable, it's not meant as the only possible 
hardware description for all pieces of the system.
Jean-Francois Moine Nov. 30, 2016, 10:44 a.m. UTC | #10
On Wed, 30 Nov 2016 11:52:25 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Jean-François,
> 
> On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
> > On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
> > >> Well, I don't see what this connector can be.
> > >> May you give me a DT example?
> > > 
> > > Sure.
> > > 
> > > arch/arm/boot/dts/r8a7791-koelsch.dts
> > > 
> > >         /* HDMI encoder */
	[snip]
> > >         /* HDMI connector */
> > >         
> > >         hdmi-out {
> > >                 compatible = "hdmi-connector";
> > >                 type = "a";
> > >                 
> > >                 port {
> > >                         hdmi_con: endpoint {
> > >                                 remote-endpoint = <&adv7511_out>;
> > >                         };
> > >                 };
> > >         };
	[snip]
> > - what does the software do with the connector type?
> 
> That's up to the software to decide, the DT bindings should describe the 
> hardware in the most accurate and usable way for the OS as possible. One of my 
> longer term goals is to add connector drivers to handle DDC and HPD when 
> they're not handled by the encoder (they are in the above example).
> 
> If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD 
> to a GPIO, we would have
> 
> 	hdmi-out {
> 		compatible = "hdmi-connector";
> 		type = "a";
> 		/* I2C bus and GPIO references are made up for the example */
> 		ddc-i2c-bus = <&i2c4>;
> 		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
> 
> 		port {
> 			hdmi_con: endpoint {
> 				remote-endpoint = <&adv7511_out>;
> 			};
> 		};
> 	};
> 
> and both HPD and EDID reading should be handled by the connector driver.
	[snip]

Hi Laurent,

OK. I understand. This connector complexity should be added in all DTs,
and the same code would be used.

Actually, for component binding, I use drm_of_component_probe():

- from the DRM master, loop on the "ports" phandle array and bind the
  CRTCs,

- for each CRTC, loop on the first remote port level and bind the
  encoders/connectors

Now, this should be:

- from the DRM master, loop on the first remote ports level and bind
  the CRTCs,

- for each CRTC, loop on the second remote port level and bind the
  encoders (and bridges?),

- for each encoder, loop on the third remote port level and bind the
  connectors.

Then, it would be nice to have a generic function for doing this job.

Otherwise, from your description:

> 	hdmi-out {
> 		compatible = "hdmi-connector";
> 		type = "a";
> 		/* I2C bus and GPIO references are made up for the example */
> 		ddc-i2c-bus = <&i2c4>;
> 		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>

the "hdmi-connector" is a big piece of software. It must handle a lot
of more and more exotic connectors.
So, I hope that you have written a "simple-hdmi-connector" which does
nothing but setting the connector type.
Where is it?
Icenowy Zheng Nov. 30, 2016, 5:24 p.m. UTC | #11
30.11.2016, 17:28, "Jean-Francois Moine" <moinejf@free.fr>:
> On Wed, 30 Nov 2016 10:20:21 +0200
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>
>>  > Well, I don't see what this connector can be.
>>  > May you give me a DT example?
>>
>>  Sure.
>>
>>  arch/arm/boot/dts/r8a7791-koelsch.dts
>>
>>          /* HDMI encoder */
>>
>>          hdmi@39 {
>>                  compatible = "adi,adv7511w";
>>                  reg = <0x39>;
>>                  interrupt-parent = <&gpio3>;
>>                  interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
>>
>>                  adi,input-depth = <8>;
>>                  adi,input-colorspace = "rgb";
>>                  adi,input-clock = "1x";
>>                  adi,input-style = <1>;
>>                  adi,input-justification = "evenly";
>>
>>                  ports {
>>                          #address-cells = <1>;
>>                          #size-cells = <0>;
>>
>>                          port@0 {
>>                                  reg = <0>;
>>                                  adv7511_in: endpoint {
>>                                          remote-endpoint = <&du_out_rgb>;
>>                                  };
>>                          };
>>
>>                          port@1 {
>>                                  reg = <1>;
>>                                  adv7511_out: endpoint {
>>                                          remote-endpoint = <&hdmi_con>;
>>                                  };
>>                          };
>>                  };
>>          };
>>
>>          /* HDMI connector */
>>
>>          hdmi-out {
>>                  compatible = "hdmi-connector";
>>                  type = "a";
>>
>>                  port {
>>                          hdmi_con: endpoint {
>>                                  remote-endpoint = <&adv7511_out>;
>>                          };
>>                  };
>>          };
>
> Hi Laurent,
>
> Sorry for I don't see the interest:
> - it is obvious that the HDMI connector is a 'hdmi-connector'!

Yes, it means the wire between the HDMI pins on the SoC and the connector ;-)

> - the physical connector type may be changed on any board by a soldering
>   iron or a connector to connector cable.

I can always alter the devices on a board ;-)

But I should also alter the dt after altering the board.

> - what does the software do with the connector type?
> - why not to put the connector information in the HDMI device?
>
> And, if I follow you, the graph of ports could also be used to describe
> the way the various parts of the SoCs are powered, to describe the pin
> connections, to describe the USB connectors, to describe the board
> internal hubs and bridges...
>
> --
> Ken ar c'hentañ | ** Breizh ha Linux atav! **
> Jef | http://moinejf.free.fr/
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Icenowy Zheng Nov. 30, 2016, 5:33 p.m. UTC | #12
30.11.2016, 18:44, "Jean-Francois Moine" <moinejf@free.fr>:
> On Wed, 30 Nov 2016 11:52:25 +0200
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>
>>  Hi Jean-François,
>>
>>  On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
>>  > On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
>>  > >> Well, I don't see what this connector can be.
>>  > >> May you give me a DT example?
>>  > >
>>  > > Sure.
>>  > >
>>  > > arch/arm/boot/dts/r8a7791-koelsch.dts
>>  > >
>>  > > /* HDMI encoder */
>
>         [snip]
>>  > > /* HDMI connector */
>>  > >
>>  > > hdmi-out {
>>  > > compatible = "hdmi-connector";
>>  > > type = "a";
>>  > >
>>  > > port {
>>  > > hdmi_con: endpoint {
>>  > > remote-endpoint = <&adv7511_out>;
>>  > > };
>>  > > };
>>  > > };
>
>         [snip]
>>  > - what does the software do with the connector type?
>>
>>  That's up to the software to decide, the DT bindings should describe the
>>  hardware in the most accurate and usable way for the OS as possible. One of my
>>  longer term goals is to add connector drivers to handle DDC and HPD when
>>  they're not handled by the encoder (they are in the above example).
>>
>>  If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD
>>  to a GPIO, we would have
>>
>>          hdmi-out {
>>                  compatible = "hdmi-connector";
>>                  type = "a";
>>                  /* I2C bus and GPIO references are made up for the example */
>>                  ddc-i2c-bus = <&i2c4>;
>>                  hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
>>
>>                  port {
>>                          hdmi_con: endpoint {
>>                                  remote-endpoint = <&adv7511_out>;
>>                          };
>>                  };
>>          };
>>
>>  and both HPD and EDID reading should be handled by the connector driver.
>
>         [snip]
>
> Hi Laurent,
>
> OK. I understand. This connector complexity should be added in all DTs,
> and the same code would be used.
>
> Actually, for component binding, I use drm_of_component_probe():
>
> - from the DRM master, loop on the "ports" phandle array and bind the
>   CRTCs,
>
> - for each CRTC, loop on the first remote port level and bind the
>   encoders/connectors
>
> Now, this should be:
>
> - from the DRM master, loop on the first remote ports level and bind
>   the CRTCs,
>
> - for each CRTC, loop on the second remote port level and bind the
>   encoders (and bridges?),
>
> - for each encoder, loop on the third remote port level and bind the
>   connectors.
>
> Then, it would be nice to have a generic function for doing this job.
>
> Otherwise, from your description:
>
>>          hdmi-out {
>>                  compatible = "hdmi-connector";
>>                  type = "a";
>>                  /* I2C bus and GPIO references are made up for the example */
>>                  ddc-i2c-bus = <&i2c4>;
>>                  hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
>
> the "hdmi-connector" is a big piece of software. It must handle a lot
> of more and more exotic connectors.
> So, I hope that you have written a "simple-hdmi-connector" which does
> nothing but setting the connector type.
> Where is it?

I suddenly thought about something...

If a DVI connector instead of a HDMI connector is soldered, how should such a
device tree be written?

How about solder a HDMI-to-VGA bridge on the board? (Maybe there should be
"dumb-hdmi-dvi-bridge" and "dumb-hdmi-vga-bridge" drivers?)

>
> --
> Ken ar c'hentañ | ** Breizh ha Linux atav! **
> Jef | http://moinejf.free.fr/
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Maxime Ripard Dec. 1, 2016, 8:55 a.m. UTC | #13
On Thu, Dec 01, 2016 at 01:33:30AM +0800, Icenowy Zheng wrote:
> >>          hdmi-out {
> >>                  compatible = "hdmi-connector";
> >>                  type = "a";
> >>                  /* I2C bus and GPIO references are made up for the example */
> >>                  ddc-i2c-bus = <&i2c4>;
> >>                  hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
> >
> > the "hdmi-connector" is a big piece of software. It must handle a lot
> > of more and more exotic connectors.
> > So, I hope that you have written a "simple-hdmi-connector" which does
> > nothing but setting the connector type.
> > Where is it?
> 
> I suddenly thought about something...
> 
> If a DVI connector instead of a HDMI connector is soldered, how
> should such a device tree be written?

Use a dvi-connector instead :)

> How about solder a HDMI-to-VGA bridge on the board? (Maybe there
> should be "dumb-hdmi-dvi-bridge" and "dumb-hdmi-vga-bridge"
> drivers?)

It probably wouldn't be dumb, but yeah, it would definitely be a
bridge instead of the connector.

Maxime
Laurent Pinchart Dec. 1, 2016, 10:41 a.m. UTC | #14
On Thursday 01 Dec 2016 09:55:20 Maxime Ripard wrote:
> On Thu, Dec 01, 2016 at 01:33:30AM +0800, Icenowy Zheng wrote:
> >>>          hdmi-out {
> >>>                  compatible = "hdmi-connector";
> >>>                  type = "a";
> >>>                  /* I2C bus and GPIO references are made up for the
> >>> example */ ddc-i2c-bus = <&i2c4>;
> >>>                  hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
> >> 
> >> the "hdmi-connector" is a big piece of software. It must handle a lot
> >> of more and more exotic connectors.
> >> So, I hope that you have written a "simple-hdmi-connector" which does
> >> nothing but setting the connector type.
> >> Where is it?
> > 
> > I suddenly thought about something...
> > 
> > If a DVI connector instead of a HDMI connector is soldered, how
> > should such a device tree be written?
> 
> Use a dvi-connector instead :)

The HDMI encoder DT node doesn't (and certainly shouldn't) report what type of 
connector is mounted on the board. Having a connector node in DT makes the 
connector type available to the system, allowing the DRM driver to expose the 
right connector type to userspace (it would be confusing for the user to 
report DRM_MODE_CONNECTOR_HDMIA for a DVI connector).

> > How about solder a HDMI-to-VGA bridge on the board? (Maybe there
> > should be "dumb-hdmi-dvi-bridge" and "dumb-hdmi-vga-bridge"
> > drivers?)
> 
> It probably wouldn't be dumb, but yeah, it would definitely be a
> bridge instead of the connector.
Jean-Francois Moine Dec. 1, 2016, 11:30 a.m. UTC | #15
On Thu, 01 Dec 2016 12:41:20 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> > > If a DVI connector instead of a HDMI connector is soldered, how
> > > should such a device tree be written?
> > 
> > Use a dvi-connector instead :)
> 
> The HDMI encoder DT node doesn't (and certainly shouldn't) report what type of 
> connector is mounted on the board. Having a connector node in DT makes the 
> connector type available to the system, allowing the DRM driver to expose the 
> right connector type to userspace (it would be confusing for the user to 
> report DRM_MODE_CONNECTOR_HDMIA for a DVI connector).

The connector type, HDMI or DVI, is known by the EDID.
The user is not interested by the software indication of the connector
type: (s)he knows it because (s)he connected him/herself the display
device.
And the DRM driver does not do anything from the knowledge of the
connector type in the DT. Only the EDID may tell if the display device
may do audio streaming (direct HDMI with audio capability) or not
(direct HDMI without audio, HDMI to DVI cable, DVI physical connector).
Laurent Pinchart Dec. 1, 2016, 11:44 a.m. UTC | #16
On Thursday 01 Dec 2016 12:30:09 Jean-Francois Moine wrote:
> On Thu, 01 Dec 2016 12:41:20 +0200 Laurent Pinchart wrote:
> >>> If a DVI connector instead of a HDMI connector is soldered, how
> >>> should such a device tree be written?
> >> 
> >> Use a dvi-connector instead :)
> > 
> > The HDMI encoder DT node doesn't (and certainly shouldn't) report what
> > type of connector is mounted on the board. Having a connector node in DT
> > makes the connector type available to the system, allowing the DRM driver
> > to expose the right connector type to userspace (it would be confusing
> > for the user to report DRM_MODE_CONNECTOR_HDMIA for a DVI connector).
> 
> The connector type, HDMI or DVI, is known by the EDID.
> The user is not interested by the software indication of the connector
> type: (s)he knows it because (s)he connected him/herself the display
> device.

That's not correct. The connector type reported by DRM to userspace can be 
used as a hint to information the user about connectors. Displaying a "Please 
connect a monitor to the HDMI connector" is confusing when the system has a 
DVI connector.

> And the DRM driver does not do anything from the knowledge of the
> connector type in the DT. Only the EDID may tell if the display device
> may do audio streaming (direct HDMI with audio capability) or not
> (direct HDMI without audio, HDMI to DVI cable, DVI physical connector).
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
new file mode 100644
index 0000000..1e107cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
@@ -0,0 +1,56 @@ 
+Allwinner HDMI Transmitter
+==========================
+
+The Allwinner HDMI transmitters are included in the SoCs.
+They support audio and video.
+
+Required properties:
+ - compatible : should be one of
+		"allwinner,sun8i-a83t-hdmi"
+		"allwinner,sun8i-h3-hdmi"
+ - reg: base address and size of the I/O memory
+ - clocks : phandles to the HDMI clocks as described in
+	Documentation/devicetree/bindings/clock/clock-bindings.txt
+ - clock-names : must be
+		"bus" : bus gate
+		"clock" : streaming clock
+		"ddc-clock" : DDC clock
+ - resets : One or two phandles to the HDMI resets
+ - reset-names : when 2 phandles, must be
+		"hdmi0" and "hdmi1"
+ - #address-cells : should be <1>
+ - #size-cells : should be <0>
+
+Required nodes:
+ - port: Audio and video input port nodes with endpoint definitions
+	as defined in Documentation/devicetree/bindings/graph.txt.
+	port@0 is video and port@1 is audio.
+
+Example:
+
+	hdmi: hdmi@01ee0000 {
+		compatible = "allwinner,sun8i-a83t-hdmi";
+		reg = <0x01ee0000 0x20000>;
+		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
+			 <&ccu CLK_HDMI_DDC>;
+		clock-names = "bus", "clock", "ddc-clock";
+		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
+		reset-names = "hdmi0", "hdmi1";
+		pinctrl-names = "default";
+		pinctrl-0 = <&hdmi_pins_a>;
+		status = "disabled";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@0 {			/* video */
+			reg = <0>;
+			hdmi_tcon1: endpoint {
+				remote-endpoint = <&tcon1_hdmi>;
+			};
+		};
+		port@1 {			/* audio */
+			reg = <1>;
+			hdmi_i2s2: endpoint {
+				remote-endpoint = <&i2s2_hdmi>;
+			};
+		};
+	};