Hi Laurent, On Mon, 2024-04-15 at 19:35 +0300, Laurent Pinchart wrote: > On Mon, Apr 15, 2024 at 04:08:10PM +0100, Kieran Bingham wrote: > > Hi Chris, > > > > > > Quoting Christopher Obbard (2024-04-15 12:41:27) > > > Enable the HDMI output on the Debix Model A SBC, using the HDMI encoder > > > present in the i.MX8MP SoC. > > > > Aha, you beat me to it. I have a commit locally (Dated 2022-09-06) but > > not sent because I didn't realise the HDMI support finally got upstream > > \o/ > > > > > Signed-off-by: Christopher Obbard <chris.obbard@xxxxxxxxxxxxx> > > > --- > > > > > > .../dts/freescale/imx8mp-debix-model-a.dts | 47 +++++++++++++++++++ > > > 1 file changed, 47 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > > b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > > index 2c19766ebf09..29529c2ecac9 100644 > > > --- a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > > @@ -20,6 +20,18 @@ chosen { > > > stdout-path = &uart2; > > > }; > > > > > > + hdmi-connector { > > > + compatible = "hdmi-connector"; > > > + label = "hdmi"; > > > + type = "a"; > > > + > > > + port { > > > + hdmi_connector_in: endpoint { > > > + remote-endpoint = <&hdmi_tx_out>; > > > + }; > > > + }; > > > + }; > > > + > > > > Interesting. My patch missed this. But it looks correct. > > > > > leds { > > > compatible = "gpio-leds"; > > > pinctrl-names = "default"; > > > @@ -94,6 +106,28 @@ ethphy0: ethernet-phy@0 { /* RTL8211E */ > > > }; > > > }; > > > > > > +&hdmi_pvi { > > > + status = "okay"; > > > +}; > > > + > > > +&hdmi_tx { > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&pinctrl_hdmi>; > > > + status = "okay"; > > > + > > > + ports { > > > + port@1 { > > > + hdmi_tx_out: endpoint { > > > + remote-endpoint = <&hdmi_connector_in>; > > > + }; > > > + }; > > > + }; > > > +}; > > > + > > > +&hdmi_tx_phy { > > > + status = "okay"; > > > +}; > > > + > > > &i2c1 { > > > clock-frequency = <400000>; > > > pinctrl-names = "default"; > > > @@ -241,6 +275,10 @@ &i2c6 { > > > status = "okay"; > > > }; > > > > > > +&lcdif3 { > > > + status = "okay"; > > > +}; > > > + > > > > Except for the addition of the connector, the above matches my patch to > > here. > > > > > &snvs_pwrkey { > > > status = "okay"; > > > }; > > > > But in my patch I have the following hunk here: (I haven't checked to > > see if this still applies on mainline, so take with a pinch of salt if > > it's not there!) > > > > > > &iomuxc { > > pinctrl-names = "default"; > > - pinctrl-0 = <&pinctrl_hog>; > > - > > - pinctrl_hog: hoggrp { > > - fsl,pins = < > > - > > MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL 0x400001c3 > > - > > MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA 0x400001c3 > > - > > MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD 0x40000019 > > - > > MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC 0x40000019 > > - >; > > - }; > > > > pinctrl_eqos: eqosgrp { > > fsl,pins = < > > MX8MP_IOMUXC_ENET_MDC__ENET_QOS_MDC > > 0x3 > > MX8MP_IOMUXC_ENET_MDIO__ENET_QOS_MDIO > > 0x3 > > > > > > > @@ -358,6 +396,15 @@ > > > MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16 0x19 > > > >; > > > }; > > > > > > + pinctrl_hdmi: hdmigrp { > > > + fsl,pins = < > > > + > > > MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL > > > 0x400001c3 > > > + > > > MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA > > > 0x400001c3 > > > + > > > MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD > > > 0x40000019 > > > + > > > MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC > > > 0x40000019 > > > + >; > > > + }; > > > + > > > > And my addition here is : > > > > > > + pinctrl_hdmi: hdmigrp { > > + fsl,pins = < > > + MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL 0 > > x1c3 > > + MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA 0 > > x1c3 > > + MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD > > 0x19 > > + MX8MP_IOMUXC_HDMI_CEC__HDMIMIX_HDMI_CEC > > 0x19 > > + >; > > + }; > > + > > > > > > I haven't looked into what the 0x40000000 does yet, but just > > highlighting the difference from the version I've been using to make use > > of HDMI so far. > > > > Does anyone else know the impact here? Otherwise I'll try to find time > > to check this later. (For some undefined term of later...) > > In drivers/pinctrl/freescale/pinctrl-imx.c, > > #define IMX_NO_PAD_CTL 0x80000000 /* no pin config need */ > #define IMX_PAD_SION 0x40000000 /* set SION */ > > The SION (Software Input ON) bit forces the input path active for the > pin. This can be used, for instance, to capture through GPIO the value > of a pin driven by a module. I'm not sure that's needed here. Thanks for the explanation, makes perfect sense. I will send a v2 without the SION bit set (e.g exactly per the hunk in Kieran's patch). Chris > > > > pinctrl_i2c1: i2c1grp { > > > fsl,pins = < > > > > > > MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL > > > 0x400001c2 >