| Message ID | 6d6653416fd87c678a80df1d422420877261c4a5.1424961754.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26/02/15 16:55, Jyri Sarha wrote: > Use new binding for the external tda19988 HDMI encoder. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > 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 @@ > > &lcdc { > status = "okay"; > + port { > + lcdc_0: endpoint@0 { > + remote-endpoint = <&hdmi_0>; > + }; > + }; > }; > > -/ { > - hdmi { > - compatible = "ti,tilcdc,slave"; > - i2c = <&i2c0>; > +&i2c0 { > + tda19988 { > + compatible = "nxp,tda998x"; > + reg = <0x70>; > pinctrl-names = "default", "off"; > pinctrl-0 = <&nxp_hdmi_bonelt_pins>; > pinctrl-1 = <&nxp_hdmi_bonelt_off_pins>; > - status = "okay"; > + > + port { > + hdmi_0: endpoint@0 { > + remote-endpoint = <&lcdc_0>; > + }; > + }; > }; > }; This is missing the output of tda998x. It should have two ports, input and output, output going to hdmi-connector. Tomi
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 <jsarha@ti.com> > > --- > > 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 @@ > > > > &lcdc { > > status = "okay"; > > + port { > > + lcdc_0: endpoint@0 { > > + remote-endpoint = <&hdmi_0>; > > + }; > > + }; > > }; > > > > -/ { > > - hdmi { > > - compatible = "ti,tilcdc,slave"; > > - i2c = <&i2c0>; > > +&i2c0 { > > + tda19988 { > > + compatible = "nxp,tda998x"; > > + reg = <0x70>; > > pinctrl-names = "default", "off"; > > pinctrl-0 = <&nxp_hdmi_bonelt_pins>; > > pinctrl-1 = <&nxp_hdmi_bonelt_off_pins>; > > - status = "okay"; > > + > > + port { > > + hdmi_0: endpoint@0 { > > + remote-endpoint = <&lcdc_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.)
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
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.
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. <snip> > 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
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 @@ &lcdc { status = "okay"; + port { + lcdc_0: endpoint@0 { + remote-endpoint = <&hdmi_0>; + }; + }; }; -/ { - hdmi { - compatible = "ti,tilcdc,slave"; - i2c = <&i2c0>; +&i2c0 { + tda19988 { + compatible = "nxp,tda998x"; + reg = <0x70>; pinctrl-names = "default", "off"; pinctrl-0 = <&nxp_hdmi_bonelt_pins>; pinctrl-1 = <&nxp_hdmi_bonelt_off_pins>; - status = "okay"; + + port { + hdmi_0: endpoint@0 { + remote-endpoint = <&lcdc_0>; + }; + }; }; };
Use new binding for the external tda19988 HDMI encoder. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- arch/arm/boot/dts/am335x-boneblack.dts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)