| Message ID | 1393590016-9361-4-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 28, 2014 at 06:12:23PM +0200, Tomi Valkeinen wrote: > On 28/02/14 17:59, Russell King - ARM Linux wrote: > > >> +dvi0: connector@0 { > >> + compatible = "dvi-connector"; > >> + label = "dvi"; > >> + > >> + i2c-bus = <&i2c3>; > >> + > >> + dvi_connector_in: endpoint { > >> + remote-endpoint = <&tfp410_out>; > >> + }; > >> +}; > > > > This looks far too simplistic. There are different classes of DVI > > connector - there is: > > > > DVI A - analogue only > > DVI D - digital only (single and dual link) > > DVI I - both (single and dual digital link) > > > > DRM at least makes a distinction between these three classes, and this > > disctinction is part of the user API. How would a display system know > > which kind of DVI connector is wired up on the board from this DT > > description? > > Yes, I think that's a valid change. But do we also need to specify > single/dual link, in addition to the three types? > > I guess the compatible string is the easiest way for differentation, at > least for the three main types, i.e. "dvi-d-connector" etc. > > "dvi-d-1l-connector" and "dvi-d-2l-connector" for the single/dual link? > That looks a bit funny. maybe like this: Required Properties: - compatible: should contain one of the following: * "dvi-d-connector" * "dvi-a-connector" * "dvi-i-connector" Optional Properties: - dual-link: Should be set for dual-link capable connectors -- Sebastian
On 28/02/14 15:43, Philipp Zabel wrote: > Hi Tomi, > > Am Freitag, den 28.02.2014, 14:20 +0200 schrieb Tomi Valkeinen: >> Add DT binding documentation for DVI Connector. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> Reviewed-by: Archit Taneja <archit@ti.com> >> --- >> .../devicetree/bindings/video/dvi-connector.txt | 26 ++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/video/dvi-connector.txt >> >> diff --git a/Documentation/devicetree/bindings/video/dvi-connector.txt b/Documentation/devicetree/bindings/video/dvi-connector.txt >> new file mode 100644 >> index 000000000000..6a0aff866c78 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/video/dvi-connector.txt >> @@ -0,0 +1,26 @@ >> +DVI Connector >> +============== >> + >> +Required properties: >> +- compatible: "dvi-connector" >> + >> +Optional properties: >> +- label: a symbolic name for the connector >> +- i2c-bus: phandle to the i2c bus that is connected to DVI DDC > > For the i.MX bindings I had called this property 'ddc', but > Documentation/devicetree/bindings/panel/simple-panel.txt already > uses 'ddc-i2c-bus'. We should definitely standardize this. I like 'ddc-i2c-bus'. Tomi
On 28/02/14 18:23, Russell King - ARM Linux wrote: >> I guess the compatible string is the easiest way for differentation, at >> least for the three main types, i.e. "dvi-d-connector" etc. >> >> "dvi-d-1l-connector" and "dvi-d-2l-connector" for the single/dual link? >> That looks a bit funny. > > I think that starts getting a tad messy: > > dvi-a-connector > dvi-d-1l-connector > dvi-d-2l-connector > dvi-i-1l-connector > dvi-i-2l-connector Yes, it's messy. Pondering this over the weekend, I think it makes sense to have just one compatible string, as all those connectors are still the same DVI connector, just different variations of the same. > That's rather a lot of compatible strings. Another possibility is: > > compatible = "dvi-connector"; > analog; > digital; > single-link; > dual-link; > > I'm debating whether "-signalling" should be on the 2nd and 3rd (or... > -signaling depending on how you prefer to spell that word.) At least > one of "analog" and/or "digital" must be specified, and if "digital" > is specified, then exactly one of "single-link" or "dual-link" should > be specified. > > So, this would mean we end up with: > > compatible = "dvi-connector"; > analog; > digital; > dual-link; > > for a DVI-I dual-link connector. Another option would be: num-links = 2; But I like your suggestion more. We could also optimize it, "digital" is extra as both "single-link" and "dual-link" mean also digital. But... I don't see much point in optimizing that way. So I agree with your suggestion as is. Tomi
On Fri, Feb 28, 2014 at 04:23:27PM +0000, Russell King - ARM Linux wrote: > On Fri, Feb 28, 2014 at 06:12:23PM +0200, Tomi Valkeinen wrote: > > On 28/02/14 17:59, Russell King - ARM Linux wrote: > > > > >> +dvi0: connector@0 { > > >> + compatible = "dvi-connector"; > > >> + label = "dvi"; > > >> + > > >> + i2c-bus = <&i2c3>; > > >> + > > >> + dvi_connector_in: endpoint { > > >> + remote-endpoint = <&tfp410_out>; > > >> + }; > > >> +}; > > > > > > This looks far too simplistic. There are different classes of DVI > > > connector - there is: > > > > > > DVI A - analogue only > > > DVI D - digital only (single and dual link) > > > DVI I - both (single and dual digital link) > > > > > > DRM at least makes a distinction between these three classes, and this > > > disctinction is part of the user API. How would a display system know > > > which kind of DVI connector is wired up on the board from this DT > > > description? > > > > Yes, I think that's a valid change. But do we also need to specify > > single/dual link, in addition to the three types? > > I would argue that as it's a difference in physical hardware, then it > should be described in DT, even if we don't use it. The reasoning is > that although we may not use it today, we may need to use it in the > future, and as we're describing what the hardware actually is - and > even in this case what pins may be present or missing on the connector, > it's unlikely to be problematical (the only problem is when someone > omits it...) If you plug a dual-link dvi screen into a soc which can do dual-link but the actual connector is cheap and doesn't have this wired up (differentiate wtf) then the kernel needs to know. Otherwise it can't correctly filter out the modes with dotclocks high enough to require dual link and the user will look at a black screen. 3.14 has a regression in drm/i915 where we've screwed this up ;-) -Daniel
On 28/02/14 18:23, Russell King - ARM Linux wrote: > That's rather a lot of compatible strings. Another possibility is: > > compatible = "dvi-connector"; > analog; > digital; > single-link; > dual-link; I made the following changes compared to the posted version. I decided to leave the "single-link" out, as it's implied if "digital" is set. Tomi @@ -6,11 +6,16 @@ Required properties: Optional properties: - label: a symbolic name for the connector -- i2c-bus: phandle to the i2c bus that is connected to DVI DDC +- ddc-i2c-bus: phandle to the i2c bus that is connected to DVI DDC +- analog: the connector has DVI analog pins +- digital: the connector has DVI digital pins +- dual-link: the connector has pins for DVI dual-link Required nodes: - Video port for DVI input +Note: One (or both) of 'analog' or 'digital' must be set. + Example ------- @@ -18,7 +23,9 @@ dvi0: connector@0 { compatible = "dvi-connector"; label = "dvi"; - i2c-bus = <&i2c3>; + digital; + + ddc-i2c-bus = <&i2c3>; dvi_connector_in: endpoint { remote-endpoint = <&tfp410_out>;
On Wed, Mar 5, 2014 at 9:41 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 28/02/14 18:23, Russell King - ARM Linux wrote: > >> That's rather a lot of compatible strings. Another possibility is: >> >> compatible = "dvi-connector"; >> analog; >> digital; >> single-link; >> dual-link; > > I made the following changes compared to the posted version. I decided > to leave the "single-link" out, as it's implied if "digital" is set. > > Tomi > > @@ -6,11 +6,16 @@ Required properties: > > Optional properties: > - label: a symbolic name for the connector > -- i2c-bus: phandle to the i2c bus that is connected to DVI DDC > +- ddc-i2c-bus: phandle to the i2c bus that is connected to DVI DDC > +- analog: the connector has DVI analog pins > +- digital: the connector has DVI digital pins > +- dual-link: the connector has pins for DVI dual-link > > Required nodes: > - Video port for DVI input > > +Note: One (or both) of 'analog' or 'digital' must be set. So dual-link needs both "digital" and "dual-link"? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 06/03/14 10:39, Geert Uytterhoeven wrote: > On Wed, Mar 5, 2014 at 9:41 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >> On 28/02/14 18:23, Russell King - ARM Linux wrote: >> >>> That's rather a lot of compatible strings. Another possibility is: >>> >>> compatible = "dvi-connector"; >>> analog; >>> digital; >>> single-link; >>> dual-link; >> >> I made the following changes compared to the posted version. I decided >> to leave the "single-link" out, as it's implied if "digital" is set. >> >> Tomi >> >> @@ -6,11 +6,16 @@ Required properties: >> >> Optional properties: >> - label: a symbolic name for the connector >> -- i2c-bus: phandle to the i2c bus that is connected to DVI DDC >> +- ddc-i2c-bus: phandle to the i2c bus that is connected to DVI DDC >> +- analog: the connector has DVI analog pins >> +- digital: the connector has DVI digital pins >> +- dual-link: the connector has pins for DVI dual-link >> >> Required nodes: >> - Video port for DVI input >> >> +Note: One (or both) of 'analog' or 'digital' must be set. > > So dual-link needs both "digital" and "dual-link"? Yes. It is extra, but it felt clearer to me to have 'digital' as a matching property for 'analog'. Alternatively we could have three options: analog; digital-single-link; digital-dual-link; My reasoning to the format I chose was basically that when a connector supports 'digital', it contains TMDS clock and TMDS data for link 1. Adding dual link to that adds only TMDS data for link 2, so the second data link is kind of an additional feature, marked with a flag. Not a very big argument, and I'm fine with other format suggestions. Tomi
Hi, Am Donnerstag, den 06.03.2014, 10:52 +0200 schrieb Tomi Valkeinen: > On 06/03/14 10:39, Geert Uytterhoeven wrote: > > On Wed, Mar 5, 2014 at 9:41 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > >> On 28/02/14 18:23, Russell King - ARM Linux wrote: > >> > >>> That's rather a lot of compatible strings. Another possibility is: > >>> > >>> compatible = "dvi-connector"; > >>> analog; > >>> digital; > >>> single-link; > >>> dual-link; > >> > >> I made the following changes compared to the posted version. I decided > >> to leave the "single-link" out, as it's implied if "digital" is set. > >> > >> Tomi > >> > >> @@ -6,11 +6,16 @@ Required properties: > >> > >> Optional properties: > >> - label: a symbolic name for the connector > >> -- i2c-bus: phandle to the i2c bus that is connected to DVI DDC > >> +- ddc-i2c-bus: phandle to the i2c bus that is connected to DVI DDC > >> +- analog: the connector has DVI analog pins > >> +- digital: the connector has DVI digital pins > >> +- dual-link: the connector has pins for DVI dual-link > >> > >> Required nodes: > >> - Video port for DVI input > >> > >> +Note: One (or both) of 'analog' or 'digital' must be set. > > > > So dual-link needs both "digital" and "dual-link"? > > Yes. It is extra, but it felt clearer to me to have 'digital' as a > matching property for 'analog'. > > Alternatively we could have three options: > > analog; > digital-single-link; > digital-dual-link; > > My reasoning to the format I chose was basically that when a connector > supports 'digital', it contains TMDS clock and TMDS data for link 1. > Adding dual link to that adds only TMDS data for link 2, so the second > data link is kind of an additional feature, marked with a flag. > > Not a very big argument, and I'm fine with other format suggestions. I'd prefer the analog / digital / dual-link variant for aesthetic reasons. But looking at other connector types, I wonder if this should be generalized even more: For HDMI/DVI (digital) single-link means one clock pair and 3 TMDS data pairs, dual-link means one clock pair and 6 data pairs. On LVDS connectors, there usually are one clock pair and three (18-bit) or four (24-bit) LVDS data pairs, in dual channel configuration two clock pairs and 6 or 8 data pairs are used. For DisplayPort there is no separate clock pair, but 1 to 4 data pairs, and MIPI DSI again has one clock pair and a one or more data pairs. There are already optional endpoint configuration properties 'data-lanes' and 'clock-lanes' for MIPI CSI-2 defined in Documentation/devicetree/bindings/media/video-interfaces.txt. Could/should this be aligned with the same? regards Philipp
On 07/03/14 16:17, Philipp Zabel wrote: > Hi, > > Am Donnerstag, den 06.03.2014, 10:52 +0200 schrieb Tomi Valkeinen: >> analog; >> digital-single-link; >> digital-dual-link; >> >> My reasoning to the format I chose was basically that when a connector >> supports 'digital', it contains TMDS clock and TMDS data for link 1. >> Adding dual link to that adds only TMDS data for link 2, so the second >> data link is kind of an additional feature, marked with a flag. >> >> Not a very big argument, and I'm fine with other format suggestions. > > I'd prefer the analog / digital / dual-link variant for aesthetic > reasons. But looking at other connector types, I wonder if this should > be generalized even more: > > For HDMI/DVI (digital) single-link means one clock pair and 3 TMDS data > pairs, dual-link means one clock pair and 6 data pairs. > > On LVDS connectors, there usually are one clock pair and three (18-bit) > or four (24-bit) LVDS data pairs, in dual channel configuration two > clock pairs and 6 or 8 data pairs are used. > > For DisplayPort there is no separate clock pair, but 1 to 4 data pairs, > and MIPI DSI again has one clock pair and a one or more data pairs. > > There are already optional endpoint configuration properties > 'data-lanes' and 'clock-lanes' for MIPI CSI-2 defined in > Documentation/devicetree/bindings/media/video-interfaces.txt. > Could/should this be aligned with the same? Hmm. Well, at least for HDMI and DP we anyway need the connector type, which tells the form factor, and it also tells the TMDS details. So, we could define the lanes in a common way, but we'd still need extra information. For MIPI DSI and (I believe) LVDS, we don't need connectors. Connectors, as described in this binding, are meant for standard hotpluggable connectors to which you can connect any device that has that same connector. Tomi
On Fri, Feb 28, 2014 at 10:25 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Freitag, den 28.02.2014, 15:59 +0000 schrieb Russell King - ARM > Linux: >> On Fri, Feb 28, 2014 at 02:20:10PM +0200, Tomi Valkeinen wrote: >> > Add DT binding documentation for DVI Connector. >> > >> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> > Reviewed-by: Archit Taneja <archit@ti.com> >> > --- >> > .../devicetree/bindings/video/dvi-connector.txt | 26 ++++++++++++++++++++++ >> > 1 file changed, 26 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/video/dvi-connector.txt >> > >> > diff --git a/Documentation/devicetree/bindings/video/dvi-connector.txt b/Documentation/devicetree/bindings/video/dvi-connector.txt >> > new file mode 100644 >> > index 000000000000..6a0aff866c78 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/video/dvi-connector.txt >> > @@ -0,0 +1,26 @@ >> > +DVI Connector >> > +============== >> > + >> > +Required properties: >> > +- compatible: "dvi-connector" >> > + >> > +Optional properties: >> > +- label: a symbolic name for the connector >> > +- i2c-bus: phandle to the i2c bus that is connected to DVI DDC >> > + >> > +Required nodes: >> > +- Video port for DVI input >> > + >> > +Example >> > +------- >> > + >> > +dvi0: connector@0 { >> > + compatible = "dvi-connector"; >> > + label = "dvi"; >> > + >> > + i2c-bus = <&i2c3>; >> > + >> > + dvi_connector_in: endpoint { >> > + remote-endpoint = <&tfp410_out>; >> > + }; >> > +}; >> >> This looks far too simplistic. There are different classes of DVI >> connector - there is: >> >> DVI A - analogue only >> DVI D - digital only (single and dual link) >> DVI I - both (single and dual digital link) >> >> DRM at least makes a distinction between these three classes, and this >> disctinction is part of the user API. How would a display system know >> which kind of DVI connector is wired up on the board from this DT >> description? > > Maybe this could be inferred from the sources connected to it. For > example a i.MX5 board with the SoC internal TV Encoder and an external > SiI902x HDMI encoder connected to the same DVI I connector: > > ipu { > port@2 { > ipu_di0_disp0: endpoint { > remote-endpoint = <&sii902x_in>; > }; > }; > port@3 { > ipu_di1_tve: endpoint { > remote-endpoint = <&tve_in>; > }; > }; > }; > > &sii902x { > compatible = "si,sii9022"; > > port@0 { > sii902x_in: endpoint { > remote-endpoint = <&ipu_di0>; > }; > }; > port@1 { > sii902x_out: endpoint { > remote-endpoint = <&dvi_d_in>; > }; > }; > }; > > &tve { > compatible = "fsl,imx53-tve"; > port@0 { > tve_in: endpoint { > remote-endpoint = <&ipu_di1>; > }; > }; > port@1 { > tve_out: endpoint { > remote-endpoint = <&dvi_a_in>; > }; > }; > }; > > dvi-connector { > compatible = "dvi-connector"; > ddc-i2c-bus = <&i2c3>; > > port { > dvi_d_in: endpoint@0 { > remote-endpoint = <&sii902x_out>; > }; > dvi_a_in: endpoint@1 { > remote-endpoint = <&tve_out>; > }; > }; > }; > > It should be possible to let the connector know that those two endpoints > are connected to a TMDS source and to a VGA source, respectively. I like this proposal over the others. Although, would dual link be a single endpoint or 2 endpoints? How would you differentiate that? The port node seems a bit pointless. Rob
On 10/03/14 23:45, Rob Herring wrote: > I like this proposal over the others. Although, would dual link be a I don't like inferring the information. With the above, you can't find out that the DVI connector has digital and analog support before all the drivers are loaded. > single endpoint or 2 endpoints? How would you differentiate that? Hmm, well endpoints for a single port are exclusive. So it's either a single port and a single endpoint, or two ports and two endpoints. I think dual link has to be single port & endpoint, as the TMDS links need to be driven together as a single bus. And dual-link is not really "two links". DVI dual-link means 1 clock lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for single-link. > The port node seems a bit pointless. There's another thread discussing the ports and endpoints. The port node represents, for example, the pins for the connection for that device. And an endpoint-endpoint link represents wires between two ports. Tomi
On 28/02/14 18:25, Philipp Zabel wrote: > dvi-connector { > compatible = "dvi-connector"; > ddc-i2c-bus = <&i2c3>; > > port { > dvi_d_in: endpoint@0 { > remote-endpoint = <&sii902x_out>; > }; > dvi_a_in: endpoint@1 { > remote-endpoint = <&tve_out>; > }; > }; > }; > > It should be possible to let the connector know that those two endpoints > are connected to a TMDS source and to a VGA source, respectively. I have not worked with boards that would have the analog part, so just wondering about the above. The above would mean that either digital or analog is in use, but not both. Was that the intention? From the connector's perspective, the analog and digital pins are separate, and I think they can be used at the same time. That kind of sounds like the analog and digital pins should be represented as separate ports. Tomi
On Tue, Mar 11, 2014 at 7:39 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 10/03/14 23:45, Rob Herring wrote: >> I like this proposal over the others. Although, would dual link be a > > I don't like inferring the information. With the above, you can't find > out that the DVI connector has digital and analog support before all the > drivers are loaded. > >> single endpoint or 2 endpoints? How would you differentiate that? > > Hmm, well endpoints for a single port are exclusive. So it's either a > single port and a single endpoint, or two ports and two endpoints. I > think dual link has to be single port & endpoint, as the TMDS links need > to be driven together as a single bus. > > And dual-link is not really "two links". DVI dual-link means 1 clock > lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for > single-link. What about having a property for the number of data lanes? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 11/03/14 10:00, Geert Uytterhoeven wrote: > On Tue, Mar 11, 2014 at 7:39 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >> On 10/03/14 23:45, Rob Herring wrote: >>> I like this proposal over the others. Although, would dual link be a >> >> I don't like inferring the information. With the above, you can't find >> out that the DVI connector has digital and analog support before all the >> drivers are loaded. >> >>> single endpoint or 2 endpoints? How would you differentiate that? >> >> Hmm, well endpoints for a single port are exclusive. So it's either a >> single port and a single endpoint, or two ports and two endpoints. I >> think dual link has to be single port & endpoint, as the TMDS links need >> to be driven together as a single bus. >> >> And dual-link is not really "two links". DVI dual-link means 1 clock >> lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for >> single-link. > > What about having a property for the number of data lanes? That was already suggested by Philipp in this thread. I don't see anything wrong with that, but I don't really see benefit either. "dual-link" is a standard term for 6 data lanes for the DVI connector. And the choices are 3 or 6 data lanes, nothing else. Tomi
Am Dienstag, den 11.03.2014, 10:04 +0200 schrieb Tomi Valkeinen: > On 11/03/14 10:00, Geert Uytterhoeven wrote: > > On Tue, Mar 11, 2014 at 7:39 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > >> On 10/03/14 23:45, Rob Herring wrote: > >>> I like this proposal over the others. Although, would dual link be a > >> > >> I don't like inferring the information. With the above, you can't find > >> out that the DVI connector has digital and analog support before all the > >> drivers are loaded. > >> > >>> single endpoint or 2 endpoints? How would you differentiate that? > >> > >> Hmm, well endpoints for a single port are exclusive. So it's either a > >> single port and a single endpoint, or two ports and two endpoints. I > >> think dual link has to be single port & endpoint, as the TMDS links need > >> to be driven together as a single bus. > >> > >> And dual-link is not really "two links". DVI dual-link means 1 clock > >> lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for > >> single-link. > > > > What about having a property for the number of data lanes? > > That was already suggested by Philipp in this thread. I don't see > anything wrong with that, but I don't really see benefit either. > "dual-link" is a standard term for 6 data lanes for the DVI connector. > And the choices are 3 or 6 data lanes, nothing else. The number of lanes of a DisplayPort connector could be 1 to 4. Also, there's dual-mode DP which can use four lanes to drive somewhat-like-HDMI single link TMDS signals. regards Philipp
diff --git a/Documentation/devicetree/bindings/video/dvi-connector.txt b/Documentation/devicetree/bindings/video/dvi-connector.txt new file mode 100644 index 000000000000..6a0aff866c78 --- /dev/null +++ b/Documentation/devicetree/bindings/video/dvi-connector.txt @@ -0,0 +1,26 @@ +DVI Connector +============== + +Required properties: +- compatible: "dvi-connector" + +Optional properties: +- label: a symbolic name for the connector +- i2c-bus: phandle to the i2c bus that is connected to DVI DDC + +Required nodes: +- Video port for DVI input + +Example +------- + +dvi0: connector@0 { + compatible = "dvi-connector"; + label = "dvi"; + + i2c-bus = <&i2c3>; + + dvi_connector_in: endpoint { + remote-endpoint = <&tfp410_out>; + }; +};