diff mbox series

[1/4] ARM: dts: am437x-gp-evm: add HDMI support

Message ID 20191125131100.9839-1-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series [1/4] ARM: dts: am437x-gp-evm: add HDMI support | expand

Commit Message

Tomi Valkeinen Nov. 25, 2019, 1:10 p.m. UTC
Add HDMI support for AM437x GP EVM. The HDMI uses SiI9022 HDMI encoder,
and is mutually exclusive with the LCD. The choice between LCD and HDMI
is made by booting either with am437x-gp-evm.dtb or
am437x-gp-evm-hdmi.dtb.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/boot/dts/Makefile               |   1 +
 arch/arm/boot/dts/am437x-gp-evm-hdmi.dts | 112 +++++++++++++++++++++++
 2 files changed, 113 insertions(+)
 create mode 100644 arch/arm/boot/dts/am437x-gp-evm-hdmi.dts

Comments

Tony Lindgren Dec. 12, 2019, 5:21 p.m. UTC | #1
Hi,

* Tomi Valkeinen <tomi.valkeinen@ti.com> [191125 05:11]:
> Add HDMI support for AM437x GP EVM. The HDMI uses SiI9022 HDMI encoder,
> and is mutually exclusive with the LCD. The choice between LCD and HDMI
> is made by booting either with am437x-gp-evm.dtb or
> am437x-gp-evm-hdmi.dtb.

So Linux kernel needs a new board device tree file to toggle a GPIO line
to switch between LCD mode and HDMI?

That does not sound very user friendly for something that's supposed
to be hot pluggabe :)

> +/* Override SelLCDorHDMI from am437x-gp-evm.dts to select HDMI */
> +&gpio5 {
> +	p8 {
> +		output-low;
> +	};
> +};

How about just leave the gpio unconfigured and document that a userspace
tool or /sys/kernel/debug/gpio is needed to toggle between the modes?

Regards,

Tony
Tony Lindgren Dec. 12, 2019, 5:31 p.m. UTC | #2
* Tony Lindgren <tony@atomide.com> [191212 17:21]:
> Hi,
> 
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [191125 05:11]:
> > Add HDMI support for AM437x GP EVM. The HDMI uses SiI9022 HDMI encoder,
> > and is mutually exclusive with the LCD. The choice between LCD and HDMI
> > is made by booting either with am437x-gp-evm.dtb or
> > am437x-gp-evm-hdmi.dtb.
> 
> So Linux kernel needs a new board device tree file to toggle a GPIO line
> to switch between LCD mode and HDMI?
> 
> That does not sound very user friendly for something that's supposed
> to be hot pluggabe :)
> 
> > +/* Override SelLCDorHDMI from am437x-gp-evm.dts to select HDMI */
> > +&gpio5 {
> > +	p8 {
> > +		output-low;
> > +	};
> > +};
> 
> How about just leave the gpio unconfigured and document that a userspace
> tool or /sys/kernel/debug/gpio is needed to toggle between the modes?

Adding also Linus Walleij to Cc in case he has some ideas here.

Anyways, I'm applying the changes to dra76-evm am57xx-idk-common
into omap-for-v5.6/dt as they have no GPIO pin limitation.

I'd like to hear comments from folks on the first two though.

Regards,

Tony
Tomi Valkeinen Dec. 13, 2019, 9:24 a.m. UTC | #3
On 12/12/2019 19:31, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [191212 17:21]:
>> Hi,
>>
>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [191125 05:11]:
>>> Add HDMI support for AM437x GP EVM. The HDMI uses SiI9022 HDMI encoder,
>>> and is mutually exclusive with the LCD. The choice between LCD and HDMI
>>> is made by booting either with am437x-gp-evm.dtb or
>>> am437x-gp-evm-hdmi.dtb.
>>
>> So Linux kernel needs a new board device tree file to toggle a GPIO line
>> to switch between LCD mode and HDMI?
>>
>> That does not sound very user friendly for something that's supposed
>> to be hot pluggabe :)

True. We've had this for a long time in the TI kernel. I don't know how to implement this better, 
except perhaps with DT overlays, but that's essentially the same method.

>>> +/* Override SelLCDorHDMI from am437x-gp-evm.dts to select HDMI */
>>> +&gpio5 {
>>> +	p8 {
>>> +		output-low;
>>> +	};
>>> +};
>>
>> How about just leave the gpio unconfigured and document that a userspace
>> tool or /sys/kernel/debug/gpio is needed to toggle between the modes?

That sounds much worse than two dts files. How does X or weston know about the gpio?

And the "external" gpio wouldn't work well with DRM. We need to add all the displays at probe time, 
so we'd have LCD and HDMI. The gpio makes one of those operable, but only the external parts. The 
display controller has just one output, and we'd have a conflict there too as both displays would be 
connected to that single output. And as the display controller driver doesn't know about the gpio, 
it would fail "randomly" for one of the displays if the other one is already enabled by the userspace.

I think the correct way would be to have DRM framework understand that we have two displays, which 
are mutually exclusive, and the display pipeline drivers would have the means to switch the gpio. 
And that the display setup could be communicated properly to the userspace, and the userspace would 
understand it. I don't think any of those exists.

So, the only good solution I have figured out is to just say that we have a single display at 
runtime, defined by the dt file.

On some boards (k2g-evm, if I recall right) we have similar HW setup, but with a physical switch. We 
use the same method there, with two dts files. Again, if I recall right, the switch setting can be 
seen by the SW, so if there's a better solution to the AM4 case, probably similar could be used with 
k2g-evm, where the drivers would react to the user changing the switch.

  Tomi
Laurent Pinchart Dec. 13, 2019, 10:42 a.m. UTC | #4
Hi Tomi,

On Fri, Dec 13, 2019 at 11:24:02AM +0200, Tomi Valkeinen wrote:
> On 12/12/2019 19:31, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [191212 17:21]:
> >> Hi,
> >>
> >> * Tomi Valkeinen <tomi.valkeinen@ti.com> [191125 05:11]:
> >>> Add HDMI support for AM437x GP EVM. The HDMI uses SiI9022 HDMI encoder,
> >>> and is mutually exclusive with the LCD. The choice between LCD and HDMI
> >>> is made by booting either with am437x-gp-evm.dtb or
> >>> am437x-gp-evm-hdmi.dtb.
> >>
> >> So Linux kernel needs a new board device tree file to toggle a GPIO line
> >> to switch between LCD mode and HDMI?
> >>
> >> That does not sound very user friendly for something that's supposed
> >> to be hot pluggabe :)
> 
> True. We've had this for a long time in the TI kernel. I don't know how to implement this better, 
> except perhaps with DT overlays, but that's essentially the same method.
> 
> >>> +/* Override SelLCDorHDMI from am437x-gp-evm.dts to select HDMI */
> >>> +&gpio5 {
> >>> +	p8 {
> >>> +		output-low;
> >>> +	};
> >>> +};
> >>
> >> How about just leave the gpio unconfigured and document that a userspace
> >> tool or /sys/kernel/debug/gpio is needed to toggle between the modes?
> 
> That sounds much worse than two dts files. How does X or weston know about the gpio?
> 
> And the "external" gpio wouldn't work well with DRM. We need to add all the displays at probe time, 
> so we'd have LCD and HDMI. The gpio makes one of those operable, but only the external parts. The 
> display controller has just one output, and we'd have a conflict there too as both displays would be 
> connected to that single output. And as the display controller driver doesn't know about the gpio, 
> it would fail "randomly" for one of the displays if the other one is already enabled by the userspace.
> 
> I think the correct way would be to have DRM framework understand that we have two displays, which 
> are mutually exclusive, and the display pipeline drivers would have the means to switch the gpio. 
> And that the display setup could be communicated properly to the userspace, and the userspace would 
> understand it. I don't think any of those exists.

Isn't this what possible_clones in drm_encoder is for ? It notifies
userspace of mutual exclusions between encoders.

> So, the only good solution I have figured out is to just say that we have a single display at 
> runtime, defined by the dt file.
> 
> On some boards (k2g-evm, if I recall right) we have similar HW setup, but with a physical switch. We 
> use the same method there, with two dts files. Again, if I recall right, the switch setting can be 
> seen by the SW, so if there's a better solution to the AM4 case, probably similar could be used with 
> k2g-evm, where the drivers would react to the user changing the switch.
Tomi Valkeinen Dec. 13, 2019, 10:56 a.m. UTC | #5
On 13/12/2019 12:42, Laurent Pinchart wrote:

>> I think the correct way would be to have DRM framework understand that we have two displays, which
>> are mutually exclusive, and the display pipeline drivers would have the means to switch the gpio.
>> And that the display setup could be communicated properly to the userspace, and the userspace would
>> understand it. I don't think any of those exists.
> 
> Isn't this what possible_clones in drm_encoder is for ? It notifies
> userspace of mutual exclusions between encoders.

Hmm, how would that work here? Isn't encoder cloning about having two encoders, which take the input 
from the same video source, and then outputting to two displays?

  Tomi
Laurent Pinchart Dec. 13, 2019, 11:42 a.m. UTC | #6
Hi Tomi,

On Fri, Dec 13, 2019 at 12:56:31PM +0200, Tomi Valkeinen wrote:
> On 13/12/2019 12:42, Laurent Pinchart wrote:
> 
> >> I think the correct way would be to have DRM framework understand that we have two displays, which
> >> are mutually exclusive, and the display pipeline drivers would have the means to switch the gpio.
> >> And that the display setup could be communicated properly to the userspace, and the userspace would
> >> understand it. I don't think any of those exists.
> > 
> > Isn't this what possible_clones in drm_encoder is for ? It notifies
> > userspace of mutual exclusions between encoders.
> 
> Hmm, how would that work here? Isn't encoder cloning about having two encoders, which take the input 
> from the same video source, and then outputting to two displays?

That's the idea. If you have one encoder for HDMI and one for the panel,
you can mark them as non-clonable, and then only one of the two can be
active at a time.
Tomi Valkeinen Dec. 13, 2019, 12:04 p.m. UTC | #7
On 13/12/2019 13:42, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Fri, Dec 13, 2019 at 12:56:31PM +0200, Tomi Valkeinen wrote:
>> On 13/12/2019 12:42, Laurent Pinchart wrote:
>>
>>>> I think the correct way would be to have DRM framework understand that we have two displays, which
>>>> are mutually exclusive, and the display pipeline drivers would have the means to switch the gpio.
>>>> And that the display setup could be communicated properly to the userspace, and the userspace would
>>>> understand it. I don't think any of those exists.
>>>
>>> Isn't this what possible_clones in drm_encoder is for ? It notifies
>>> userspace of mutual exclusions between encoders.
>>
>> Hmm, how would that work here? Isn't encoder cloning about having two encoders, which take the input
>> from the same video source, and then outputting to two displays?
> 
> That's the idea. If you have one encoder for HDMI and one for the panel,
> you can mark them as non-clonable, and then only one of the two can be
> active at a time.

We have a single DPI output from the SoC. That goes to the panel, or to SiI9022 bridge, depending on 
the GPIO switch.

So... In the DT file, we would have multiple endpoints in the same output port in DSS, one going to 
the panel, one to the SiI9022? omapdrm could then create two encoders, one abstracting the DPI 
output and the connection to the panel, one abstracting the DPI output and SiI9022?

And then someone would need to handle the GPIO, and set it based on the output used. These kind of 
gpios are always difficult, as they don't belong anywhere =).

  Tomi
Laurent Pinchart Dec. 13, 2019, 12:28 p.m. UTC | #8
Hi Tomi,

On Fri, Dec 13, 2019 at 02:04:30PM +0200, Tomi Valkeinen wrote:
> On 13/12/2019 13:42, Laurent Pinchart wrote:
> > On Fri, Dec 13, 2019 at 12:56:31PM +0200, Tomi Valkeinen wrote:
> >> On 13/12/2019 12:42, Laurent Pinchart wrote:
> >>
> >>>> I think the correct way would be to have DRM framework understand that we have two displays, which
> >>>> are mutually exclusive, and the display pipeline drivers would have the means to switch the gpio.
> >>>> And that the display setup could be communicated properly to the userspace, and the userspace would
> >>>> understand it. I don't think any of those exists.
> >>>
> >>> Isn't this what possible_clones in drm_encoder is for ? It notifies
> >>> userspace of mutual exclusions between encoders.
> >>
> >> Hmm, how would that work here? Isn't encoder cloning about having two encoders, which take the input
> >> from the same video source, and then outputting to two displays?
> > 
> > That's the idea. If you have one encoder for HDMI and one for the panel,
> > you can mark them as non-clonable, and then only one of the two can be
> > active at a time.
> 
> We have a single DPI output from the SoC. That goes to the panel, or to SiI9022 bridge, depending on 
> the GPIO switch.
> 
> So... In the DT file, we would have multiple endpoints in the same output port in DSS, one going to 
> the panel, one to the SiI9022? omapdrm could then create two encoders, one abstracting the DPI 
> output and the connection to the panel, one abstracting the DPI output and SiI9022?

That's the idea, yes.

> And then someone would need to handle the GPIO, and set it based on the output used. These kind of 
> gpios are always difficult, as they don't belong anywhere =).

https://lore.kernel.org/lkml/20191211061911.238393-5-hsinyi@chromium.org/

Still, the infrastructure in omapdrm would need quite a bit of work.
We're just about to get a helper layer for linear pipelines merged, and
we already need to go one step further :-)
Tomi Valkeinen Dec. 13, 2019, 12:33 p.m. UTC | #9
On 13/12/2019 14:28, Laurent Pinchart wrote:

>> So... In the DT file, we would have multiple endpoints in the same output port in DSS, one going to
>> the panel, one to the SiI9022? omapdrm could then create two encoders, one abstracting the DPI
>> output and the connection to the panel, one abstracting the DPI output and SiI9022?
> 
> That's the idea, yes.
> 
>> And then someone would need to handle the GPIO, and set it based on the output used. These kind of
>> gpios are always difficult, as they don't belong anywhere =).
> 
> https://lore.kernel.org/lkml/20191211061911.238393-5-hsinyi@chromium.org/
> 
> Still, the infrastructure in omapdrm would need quite a bit of work.
> We're just about to get a helper layer for linear pipelines merged, and
> we already need to go one step further :-)

Alright, sounds like this will be doable in the future. So let's drop this and the epos HDMI patches 
for now.

This does sound like quite a bit of work, as you say, so I have no idea when we can get there (on 
the omapdrm side). In the minimum we should first get the big omapdrm rework done, in order to avoid 
nasty conflicts.

Thanks for educating me =).

  Tomi
Tomi Valkeinen Dec. 13, 2019, 12:36 p.m. UTC | #10
On 12/12/2019 19:31, Tony Lindgren wrote:

> Anyways, I'm applying the changes to dra76-evm am57xx-idk-common
> into omap-for-v5.6/dt as they have no GPIO pin limitation.

Thanks!

> I'd like to hear comments from folks on the first two though.

Just to summarize the discussion, let's drop the first two patches (am4).

  Tomi
Tony Lindgren Dec. 13, 2019, 2:57 p.m. UTC | #11
* Tomi Valkeinen <tomi.valkeinen@ti.com> [191213 12:34]:
> On 13/12/2019 14:28, Laurent Pinchart wrote:
> 
> > > So... In the DT file, we would have multiple endpoints in the same output port in DSS, one going to
> > > the panel, one to the SiI9022? omapdrm could then create two encoders, one abstracting the DPI
> > > output and the connection to the panel, one abstracting the DPI output and SiI9022?
> > 
> > That's the idea, yes.
> > 
> > > And then someone would need to handle the GPIO, and set it based on the output used. These kind of
> > > gpios are always difficult, as they don't belong anywhere =).
> > 
> > https://lore.kernel.org/lkml/20191211061911.238393-5-hsinyi@chromium.org/
> > 
> > Still, the infrastructure in omapdrm would need quite a bit of work.
> > We're just about to get a helper layer for linear pipelines merged, and
> > we already need to go one step further :-)
> 
> Alright, sounds like this will be doable in the future. So let's drop this
> and the epos HDMI patches for now.

Oh OK. Sounds like no other solution is usable right now short of
separate dts files like you've done.

> This does sound like quite a bit of work, as you say, so I have no idea when
> we can get there (on the omapdrm side). In the minimum we should first get
> the big omapdrm rework done, in order to avoid nasty conflicts.
> 
> Thanks for educating me =).

Sounds a nice plan though :)

Thanks,

Tony
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index b21b3a64641a..612149069180 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -779,6 +779,7 @@  dtb-$(CONFIG_SOC_AM43XX) += \
 	am43x-epos-evm.dtb \
 	am437x-cm-t43.dtb \
 	am437x-gp-evm.dtb \
+	am437x-gp-evm-hdmi.dtb \
 	am437x-idk-evm.dtb \
 	am437x-sbc-t43.dtb \
 	am437x-sk-evm.dtb
diff --git a/arch/arm/boot/dts/am437x-gp-evm-hdmi.dts b/arch/arm/boot/dts/am437x-gp-evm-hdmi.dts
new file mode 100644
index 000000000000..580a1e3e0dcd
--- /dev/null
+++ b/arch/arm/boot/dts/am437x-gp-evm-hdmi.dts
@@ -0,0 +1,112 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+/* AM437x GP EVM with HDMI output */
+
+#include "am437x-gp-evm.dts"
+
+/delete-node/ &lcd0;
+
+/ {
+	aliases {
+		display0 = &hdmi;
+	};
+
+	hdmi: connector {
+		compatible = "hdmi-connector";
+		label = "hdmi";
+
+		type = "b";
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&sii9022_out>;
+			};
+		};
+	};
+
+	sound@1 {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "HDMI";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,bitclock-master = <&hdmi_dailink_master>;
+		simple-audio-card,frame-master = <&hdmi_dailink_master>;
+		hdmi_dailink_master: simple-audio-card,cpu {
+			sound-dai = <&mcasp1>;
+			system-clock-frequency = <24000000>;
+			system-clock-direction-out;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&sii9022>;
+		};
+	};
+
+	sii9022_mclk: sii9022_mclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <12000000>;
+	};
+};
+
+&lcd_bl {
+	status = "disabled";
+};
+
+&sound0 {
+	status = "disabled";
+};
+
+&tlv320aic3106 {
+	status = "disabled";
+};
+
+&i2c1 {
+	sii9022: sii9022@3b {
+		#sound-dai-cells = <0>;
+		compatible = "sil,sii9022";
+		reg = <0x3b>;
+
+		interrupt-parent = <&gpio3>;
+		interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
+
+		sil,i2s-data-lanes = < 0 >;
+		clocks = <&sii9022_mclk>;
+		clock-names = "mclk";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				sii9022_in: endpoint {
+					remote-endpoint = <&dpi_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				sii9022_out: endpoint {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+};
+
+&dpi_out {
+	remote-endpoint = <&sii9022_in>;
+	data-lines = <24>;
+};
+
+/* Override SelLCDorHDMI from am437x-gp-evm.dts to select HDMI */
+&gpio5 {
+	p8 {
+		output-low;
+	};
+};