[PATCH RFC 6/6] ARM: dts: am335x-boneblack: Use new binding for HDMI

[Tomi Valkeinen]

On 02/03/15 19:42, Russell King - ARM Linux wrote:
> On Mon, Mar 02, 2015 at 07:08:39PM +0200, Tomi Valkeinen wrote:
>> On 02/03/15 18:06, Russell King - ARM Linux wrote:
>>
 This is missing the output of tda998x. It should have two ports, input
 and output, output going to hdmi-connector.
>>>
>>> We don't have that kind of level of modelling in DRM right now - as far
>>> as DRM is concerned, the tda998x is both the encoder _and_ the connector
>>> since it supplies both functionalities.
>>
>> That's fine, but these are DT bindings which should reflect the
>> hardware, not the SW stack.
> 
> I still don't buy your argument.  The principle is right, but I think
> you're taking it too far.



> When we say "DT should follow the hardware" we're not talking about
> making DT be an alternative to the hardware's schematic.

I agree, and that's not what I'm suggesting. We should only model HW in
the DT when it makes sense, when it gives us something.

A HDMI connector can have (at least) two functionalities that the driver
for it may need to handle: HPD and +5V. On some SoCs/boards those are
handled by the HDMI encoder, but I have boards where they are not. So we
need to have the data somewhere, and a HDMI connector node at the end of
the video path is a logical choice.

The HDMI connector node is also a good place for a symbolic name which
can be shown to the user.

In this particular board, the HDMI encoder handles the HPD and the +5V
is always enabled, so there's no strict need to have the HDMI connector
node.

However, I still feel it's better to have the HDMI connector in .dts:

1) It's not said that a HDMI encoder always has a HDMI connector as the
next component. The next component could be a integrated panel, needing
a specific driver and there's no HDMI connector at all. Or there could
be something else, as is the case on some OMAP boards which have an ESD
protection chip (that requires controlling, i.e. a driver) between the
encoder and the connector.

2) I like that the beginning and the end of the video pipeline are
clearly defined. A video pipeline starts at a display controller, and
ends at a panel or a connector. This makes it easier to understand the
.dts as you know what to expect.

In the SW side these mean that every encoder (or whatever is doing this
stuff) should be able to handle any component after the encoder, be it a
connector, panel or something else.

If we allow leaving out the connector node, the code also needs to
handle the case when there's nothing after the encoder, which probably
means fabricating some connector data (at least if and when DRM can
handle arbitrary video pipelines).

But as I said earlier, we can do just fine without the HDMI connector
node on boards where the connector "just works". We can handle that in
the drivers with some extra code.

So if people think it's a big chore to add the connectors and don't see
the benefit in them (and they don't want the symbolic labels that could
be added there), I'm fine with having them optional.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH RFC 6/6] ARM: dts: am335x-boneblack: Use new binding for HDMI

[Tomi Valkeinen]

On 02/03/15 18:06, Russell King - ARM Linux wrote:

>> This is missing the output of tda998x. It should have two ports, input
>> and output, output going to hdmi-connector.
> 
> We don't have that kind of level of modelling in DRM right now - as far
> as DRM is concerned, the tda998x is both the encoder _and_ the connector
> since it supplies both functionalities.

That's fine, but these are DT bindings which should reflect the
hardware, not the SW stack.

> We did discuss this ages ago, but afaik no concensus was reached how to
> model physical connectors in DT, so it never moved forward in DRM (and

I don't know about consensus, but omapdss is using connectors in DT, and
they were discussed in lists, and everyone seemed to be ok with that
model (Documentation/devicetree/bindings/video/hdmi-connector.txt). If I
recall right, the only real question was how the links should be modeled
(two way, one way, something else), but that's not specific to connectors.

So while it's open how they should be implemented in the DRM, I don't
see why we couldn't/shouldn't specify them in the .dts.

That said, if and when DRM supports connectors defined in .dts, we can
just assume that if tda998x does not have an output defined in the .dts,
it's connected to a HDMI connector. So we should do just fine even if we
end up not defining the connectors at this time.

> besides, everyone seems to be off doing their own thing when it comes
> to writing DT descriptions for video hardware.)

Yep... I've been trying to push the video ports/endpoints system which
afaik covers about all the use cases that have been raised. But the
counter argument usually is that "it's too complex".

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH RFC 6/6] ARM: dts: am335x-boneblack: Use new binding for HDMI

[Russell King - ARM Linux]

On Mon, Mar 02, 2015 at 07:08:39PM +0200, Tomi Valkeinen wrote:
> On 02/03/15 18:06, Russell King - ARM Linux wrote:
> 
> >> This is missing the output of tda998x. It should have two ports, input
> >> and output, output going to hdmi-connector.
> > 
> > We don't have that kind of level of modelling in DRM right now - as far
> > as DRM is concerned, the tda998x is both the encoder _and_ the connector
> > since it supplies both functionalities.
> 
> That's fine, but these are DT bindings which should reflect the
> hardware, not the SW stack.

I still don't buy your argument.  The principle is right, but I think
you're taking it too far.

Look at ePAPR for a moment, and consider a serial port.  A serial UART
can be physically connected to a 9-pin or a 25-pin connector, which
may be male or female.  These details are not included in the DT
description.  Even when the serial port control signals are provided
by GPIOs rather than the UART, we don't model the connector - we
wrap the GPIOs directly into the UART driver.

Arguably, that's not following the hardware, it's following the software
representation - it's following the software representation of what a
serial port _should_ look like to a non-specific OS.

Consider an ethernet port.  You'll find the same thing applies - the
physical connector itself is not specified.

Yet, somehow, we're wanting to specify the physical connector for _all_
video devices?  I don't see why that should be mandatory when it's not
mandatory for other subsystems.

If we want to take this to the extreme, we should be specifying the
power connectors in DT and how they're wired up along with their
controls.  We don't though, we specify the control devices and
the function of those (eg, via a charger chip.)

To take this to the extreme, what about a device powered via PoE?
Should the PoE connector be modelled in DT?

When we say "DT should follow the hardware" we're not talking about
making DT be an alternative to the hardware's schematic.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH RFC 6/6] ARM: dts: am335x-boneblack: Use new binding for HDMI

[Russell King - ARM Linux]

On Mon, Mar 02, 2015 at 02:28:40PM +0200, Tomi Valkeinen wrote:
> On 26/02/15 16:55, Jyri Sarha wrote:
> > Use new binding for the external tda19988 HDMI encoder.
> > 
> > Signed-off-by: Jyri Sarha 
> > ---
> >  arch/arm/boot/dts/am335x-boneblack.dts | 20 +++-
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am335x-boneblack.dts 
> > b/arch/arm/boot/dts/am335x-boneblack.dts
> > index 5c42d25..eadbba3 100644
> > --- a/arch/arm/boot/dts/am335x-boneblack.dts
> > +++ b/arch/arm/boot/dts/am335x-boneblack.dts
> > @@ -68,16 +68,26 @@
> >  
> >   {
> > status = "okay";
> > +   port {
> > +   lcdc_0: endpoint at 0 {
> > +   remote-endpoint = <_0>;
> > +   };
> > +   };
> >  };
> >  
> > -/ {
> > -   hdmi {
> > -   compatible = "ti,tilcdc,slave";
> > -   i2c = <>;
> > + {
> > +   tda19988 {
> > +   compatible = "nxp,tda998x";
> > +   reg = <0x70>;
> > pinctrl-names = "default", "off";
> > pinctrl-0 = <_hdmi_bonelt_pins>;
> > pinctrl-1 = <_hdmi_bonelt_off_pins>;
> > -   status = "okay";
> > +
> > +   port {
> > +   hdmi_0: endpoint at 0 {
> > +   remote-endpoint = <_0>;
> > +   };
> > +   };
> > };
> >  };
> 
> This is missing the output of tda998x. It should have two ports, input
> and output, output going to hdmi-connector.

We don't have that kind of level of modelling in DRM right now - as far
as DRM is concerned, the tda998x is both the encoder _and_ the connector
since it supplies both functionalities.

We did discuss this ages ago, but afaik no concensus was reached how to
model physical connectors in DT, so it never moved forward in DRM (and
besides, everyone seems to be off doing their own thing when it comes
to writing DT descriptions for video hardware.)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH RFC 6/6] ARM: dts: am335x-boneblack: Use new binding for HDMI

[Tomi Valkeinen]

On 26/02/15 16:55, Jyri Sarha wrote:
> Use new binding for the external tda19988 HDMI encoder.
> 
> Signed-off-by: Jyri Sarha 
> ---
>  arch/arm/boot/dts/am335x-boneblack.dts | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am335x-boneblack.dts 
> b/arch/arm/boot/dts/am335x-boneblack.dts
> index 5c42d25..eadbba3 100644
> --- a/arch/arm/boot/dts/am335x-boneblack.dts
> +++ b/arch/arm/boot/dts/am335x-boneblack.dts
> @@ -68,16 +68,26 @@
>  
>   {
>   status = "okay";
> + port {
> + lcdc_0: endpoint at 0 {
> + remote-endpoint = <_0>;
> + };
> + };
>  };
>  
> -/ {
> - hdmi {
> - compatible = "ti,tilcdc,slave";
> - i2c = <>;
> + {
> + tda19988 {
> + compatible = "nxp,tda998x";
> + reg = <0x70>;
>   pinctrl-names = "default", "off";
>   pinctrl-0 = <_hdmi_bonelt_pins>;
>   pinctrl-1 = <_hdmi_bonelt_off_pins>;
> - status = "okay";
> +
> + port {
> + hdmi_0: endpoint at 0 {
> + remote-endpoint = <_0>;
> + };
> + };
>   };
>  };

This is missing the output of tda998x. It should have two ports, input
and output, output going to hdmi-connector.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH RFC 6/6] ARM: dts: am335x-boneblack: Use new binding for HDMI

[Jyri Sarha]

Use new binding for the external tda19988 HDMI encoder.

Signed-off-by: Jyri Sarha 
---
 arch/arm/boot/dts/am335x-boneblack.dts | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts 
b/arch/arm/boot/dts/am335x-boneblack.dts
index 5c42d25..eadbba3 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -68,16 +68,26 @@

  {
status = "okay";
+   port {
+   lcdc_0: endpoint at 0 {
+   remote-endpoint = <_0>;
+   };
+   };
 };

-/ {
-   hdmi {
-   compatible = "ti,tilcdc,slave";
-   i2c = <>;
+ {
+   tda19988 {
+   compatible = "nxp,tda998x";
+   reg = <0x70>;
pinctrl-names = "default", "off";
pinctrl-0 = <_hdmi_bonelt_pins>;
pinctrl-1 = <_hdmi_bonelt_off_pins>;
-   status = "okay";
+
+   port {
+   hdmi_0: endpoint at 0 {
+   remote-endpoint = <_0>;
+   };
+   };
};
 };

-- 
1.9.1