diff mbox series

[v1,1/1] arm64: dts: imx8mp-debix-model-a: Add HDMI output support

Message ID 20240415114135.25473-2-chris.obbard@collabora.com (mailing list archive)
State New, archived
Headers show
Series Enable HDMI output support on the Debix Model A SBC | expand

Commit Message

Christopher Obbard April 15, 2024, 11:41 a.m. UTC
Enable the HDMI output on the Debix Model A SBC, using the HDMI encoder
present in the i.MX8MP SoC.

Signed-off-by: Christopher Obbard <chris.obbard@collabora.com>
---

 .../dts/freescale/imx8mp-debix-model-a.dts    | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

Kieran Bingham April 15, 2024, 3:08 p.m. UTC | #1
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@collabora.com>
> ---
> 
>  .../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	0x1c3
+			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA	0x1c3
+			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...)

--
Kieran


>         pinctrl_i2c1: i2c1grp {
>                 fsl,pins = <
>                         MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL                                 0x400001c2
> -- 
> 2.43.0
>
Laurent Pinchart April 15, 2024, 4:35 p.m. UTC | #2
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@collabora.com>
> > ---
> > 
> >  .../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	0x1c3
> +			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA	0x1c3
> +			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.

> >         pinctrl_i2c1: i2c1grp {
> >                 fsl,pins = <
> >                         MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL                                 0x400001c2
Christopher Obbard April 15, 2024, 5:07 p.m. UTC | #3
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@collabora.com>
> > > ---
> > > 
> > >  .../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
>
Laurent Pinchart June 8, 2024, 3:02 p.m. UTC | #4
On Mon, Apr 15, 2024 at 06:07:24PM +0100, Christopher Obbard wrote:
> 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:
> > > 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@collabora.com>
> > > > ---
> > > > 
> > > >  .../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).

I'd like to get this merged in v6.11. If you don't have time to send a
v2, I'm happy resending our version of the patch instead :-)

> > > >         pinctrl_i2c1: i2c1grp {
> > > >                 fsl,pins = <
> > > >                        
> > > > MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL                                
> > > > 0x400001c2
Christopher Obbard June 10, 2024, 10:55 p.m. UTC | #5
Hi Laurent,

On Sat, 2024-06-08 at 18:02 +0300, Laurent Pinchart wrote:
> On Mon, Apr 15, 2024 at 06:07:24PM +0100, Christopher Obbard wrote:
> > 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:
> > > > 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@collabora.com>
> > > > > ---
> > > > > 
> > > > >  .../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).
> 
> I'd like to get this merged in v6.11. If you don't have time to send a
> v2, I'm happy resending our version of the patch instead :-)

I've just dusted the board off and will send v2 shortly after testing.

Thanks for the reminder :-).

> 
> > > > >         pinctrl_i2c1: i2c1grp {
> > > > >                 fsl,pins = <
> > > > >                        
> > > > > MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL                                
> > > > > 0x400001c2
> 
> -- 
> Regards,
> 
> Laurent Pinchart
diff mbox series

Patch

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>;
+			};
+		};
+	};
+
 	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";
+};
+
 &snvs_pwrkey {
 	status = "okay";
 };
@@ -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
+		>;
+	};
+
 	pinctrl_i2c1: i2c1grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL					0x400001c2