linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
@ 2022-09-01 15:40 Max Krummenacher
  2022-09-01 15:40 ` [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality Max Krummenacher
  2022-09-01 18:07 ` [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Laurent Pinchart
  0 siblings, 2 replies; 10+ messages in thread
From: Max Krummenacher @ 2022-09-01 15:40 UTC (permalink / raw)
  To: Max Krummenacher
  Cc: Fabio Estevam, Krzysztof Kozlowski, Laurent Pinchart,
	Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team,
	Philippe Schenker, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-arm-kernel, linux-kernel

From: Max Krummenacher <max.krummenacher@toradex.com>

Add the hdmi connector present on the dsi to hdmi adapter now
required by the upstream lontium bridge driver.
The dsi to hdmi adapter is enabled in an device tree overlay.

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>

---

 arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
index 76cc89296150..bd84a0d135dc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
@@ -59,6 +59,14 @@ button-wakeup {
 		};
 	};
 
+	hdmi_connector: hdmi-connector {
+		compatible = "hdmi-connector";
+		ddc-i2c-bus = <&i2c2>;
+		label = "hdmi";
+		type = "a";
+		status = "disabled";
+	};
+
 	/* Carrier Board Supplies */
 	reg_1p8v: regulator-1p8v {
 		compatible = "regulator-fixed";
-- 
2.35.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality
  2022-09-01 15:40 [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Max Krummenacher
@ 2022-09-01 15:40 ` Max Krummenacher
  2022-09-01 18:10   ` Laurent Pinchart
  2022-09-01 18:07 ` [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Laurent Pinchart
  1 sibling, 1 reply; 10+ messages in thread
From: Max Krummenacher @ 2022-09-01 15:40 UTC (permalink / raw)
  To: Max Krummenacher
  Cc: Fabio Estevam, Krzysztof Kozlowski, Laurent Pinchart,
	Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team,
	Philippe Schenker, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-arm-kernel, linux-kernel

From: Max Krummenacher <max.krummenacher@toradex.com>

Add a panel-lvds node and use the correct dsi to lvds chip name.
Both to be later extended in a dt overlay according to the exact
board HW configuration.

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---

 arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
index bd84a0d135dc..a3e20c7add3e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
@@ -67,6 +67,13 @@ hdmi_connector: hdmi-connector {
 		status = "disabled";
 	};
 
+	panel_lvds: panel-lvds {
+		compatible = "panel-lvds";
+		backlight = <&backlight>;
+		data-mapping = "vesa-24";
+		status = "disabled";
+	};
+
 	/* Carrier Board Supplies */
 	reg_1p8v: regulator-1p8v {
 		compatible = "regulator-fixed";
@@ -690,8 +697,8 @@ gpio_expander_21: gpio-expander@21 {
 		status = "disabled";
 	};
 
-	lvds_ti_sn65dsi83: bridge@2c {
-		compatible = "ti,sn65dsi83";
+	lvds_ti_sn65dsi84: bridge@2c {
+		compatible = "ti,sn65dsi84";
 		/* Verdin GPIO_9_DSI (SN65DSI84 IRQ, SODIMM 17, unused) */
 		/* Verdin GPIO_10_DSI (SODIMM 21) */
 		enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
-- 
2.35.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-01 15:40 [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Max Krummenacher
  2022-09-01 15:40 ` [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality Max Krummenacher
@ 2022-09-01 18:07 ` Laurent Pinchart
  2022-09-02 15:57   ` Francesco Dolcini
  1 sibling, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2022-09-01 18:07 UTC (permalink / raw)
  To: Max Krummenacher
  Cc: Max Krummenacher, Fabio Estevam, Krzysztof Kozlowski,
	Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team,
	Philippe Schenker, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-arm-kernel, linux-kernel

Hi Max,

Thank you for the patch.

On Thu, Sep 01, 2022 at 05:40:50PM +0200, Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> Add the hdmi connector present on the dsi to hdmi adapter now
> required by the upstream lontium bridge driver.
> The dsi to hdmi adapter is enabled in an device tree overlay.

Shouldn't the connector also be in the overlay ? There's certainly no
physical HDMI connector on the i.MX8MP Verdin SoM :-)

> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> 
> ---
> 
>  arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> index 76cc89296150..bd84a0d135dc 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> @@ -59,6 +59,14 @@ button-wakeup {
>  		};
>  	};
>  
> +	hdmi_connector: hdmi-connector {
> +		compatible = "hdmi-connector";
> +		ddc-i2c-bus = <&i2c2>;
> +		label = "hdmi";
> +		type = "a";
> +		status = "disabled";
> +	};
> +
>  	/* Carrier Board Supplies */
>  	reg_1p8v: regulator-1p8v {
>  		compatible = "regulator-fixed";

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality
  2022-09-01 15:40 ` [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality Max Krummenacher
@ 2022-09-01 18:10   ` Laurent Pinchart
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2022-09-01 18:10 UTC (permalink / raw)
  To: Max Krummenacher
  Cc: Max Krummenacher, Fabio Estevam, Krzysztof Kozlowski,
	Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team,
	Philippe Schenker, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-arm-kernel, linux-kernel

Hi Max,

Thank you for the patch.

On Thu, Sep 01, 2022 at 05:40:51PM +0200, Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> Add a panel-lvds node and use the correct dsi to lvds chip name.
> Both to be later extended in a dt overlay according to the exact
> board HW configuration.

Same as with patch 1/2, this doesn't look right. The panel isn't part of
the Verdin SoM, it's not even part of the base board. It should be moved
to an overlay, and the SN65DSI83, which I understand isn't on the SoM
either, should be moved somewhere else too..

> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> ---
> 
>  arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> index bd84a0d135dc..a3e20c7add3e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> @@ -67,6 +67,13 @@ hdmi_connector: hdmi-connector {
>  		status = "disabled";
>  	};
>  
> +	panel_lvds: panel-lvds {
> +		compatible = "panel-lvds";
> +		backlight = <&backlight>;
> +		data-mapping = "vesa-24";
> +		status = "disabled";
> +	};
> +
>  	/* Carrier Board Supplies */
>  	reg_1p8v: regulator-1p8v {
>  		compatible = "regulator-fixed";
> @@ -690,8 +697,8 @@ gpio_expander_21: gpio-expander@21 {
>  		status = "disabled";
>  	};
>  
> -	lvds_ti_sn65dsi83: bridge@2c {
> -		compatible = "ti,sn65dsi83";
> +	lvds_ti_sn65dsi84: bridge@2c {
> +		compatible = "ti,sn65dsi84";
>  		/* Verdin GPIO_9_DSI (SN65DSI84 IRQ, SODIMM 17, unused) */
>  		/* Verdin GPIO_10_DSI (SODIMM 21) */
>  		enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-01 18:07 ` [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Laurent Pinchart
@ 2022-09-02 15:57   ` Francesco Dolcini
  2022-09-03  0:24     ` Laurent Pinchart
  0 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2022-09-02 15:57 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Max Krummenacher, Max Krummenacher, Fabio Estevam,
	Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team,
	Pengutronix Kernel Team, Philippe Schenker, Rob Herring,
	Sascha Hauer, Shawn Guo, devicetree, linux-arm-kernel,
	linux-kernel

Hello Laurent,
answering here for both patches (1/2 and 2/2).

On Thu, Sep 01, 2022 at 09:07:49PM +0300, Laurent Pinchart wrote:
> On Thu, Sep 01, 2022 at 05:40:50PM +0200, Max Krummenacher wrote:
> > From: Max Krummenacher <max.krummenacher@toradex.com>
> > 
> > Add the hdmi connector present on the dsi to hdmi adapter now
> > required by the upstream lontium bridge driver.
> > The dsi to hdmi adapter is enabled in an device tree overlay.
> 
> Shouldn't the connector also be in the overlay ? There's certainly no
> physical HDMI connector on the i.MX8MP Verdin SoM :-)

Toradex DTS include and overlay files structure so far has been a little
bit different and not following the expectation you just stated here,
you can just check the current *toradex*dts* files and you'll see that there
is other stuff that is not strictly part of the module.

Copying from a previous email thread on a very similar discussion [0]
some of the reasons:

 - The SoM dtsi representing not only the functionality implemented into
   the SoM, but the whole connector pinout to the carrier makes very easy
   to just include a different som.dtsi in the carrier board dts and just
   switch SoM, for example from a colibri-imx6 to a colibri-imx7.
 - We avoid code duplication

This is working for us pretty well so far and the majority of the users
of ours modules rely on this structure, we would prefer not to change that.

Francesco

[0] https://lore.kernel.org/all/20220413094449.GB118560@francesco-nb.int.toradex.com/


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-02 15:57   ` Francesco Dolcini
@ 2022-09-03  0:24     ` Laurent Pinchart
  2022-09-03 12:47       ` Francesco Dolcini
  0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2022-09-03  0:24 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Max Krummenacher, Max Krummenacher, Fabio Estevam,
	Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team,
	Pengutronix Kernel Team, Philippe Schenker, Rob Herring,
	Sascha Hauer, Shawn Guo, devicetree, linux-arm-kernel,
	linux-kernel

Hi Francesco,

On Fri, Sep 02, 2022 at 05:57:20PM +0200, Francesco Dolcini wrote:
> Hello Laurent,
> answering here for both patches (1/2 and 2/2).
> 
> On Thu, Sep 01, 2022 at 09:07:49PM +0300, Laurent Pinchart wrote:
> > On Thu, Sep 01, 2022 at 05:40:50PM +0200, Max Krummenacher wrote:
> > > From: Max Krummenacher <max.krummenacher@toradex.com>
> > > 
> > > Add the hdmi connector present on the dsi to hdmi adapter now
> > > required by the upstream lontium bridge driver.
> > > The dsi to hdmi adapter is enabled in an device tree overlay.
> > 
> > Shouldn't the connector also be in the overlay ? There's certainly no
> > physical HDMI connector on the i.MX8MP Verdin SoM :-)
> 
> Toradex DTS include and overlay files structure so far has been a little
> bit different and not following the expectation you just stated here,
> you can just check the current *toradex*dts* files and you'll see that there
> is other stuff that is not strictly part of the module.
> 
> Copying from a previous email thread on a very similar discussion [0]
> some of the reasons:
> 
>  - The SoM dtsi representing not only the functionality implemented into
>    the SoM, but the whole connector pinout to the carrier makes very easy
>    to just include a different som.dtsi in the carrier board dts and just
>    switch SoM, for example from a colibri-imx6 to a colibri-imx7.

That's fine, but I don't see how that's related to the issue at hand.
The DSI to HDMI bridge wouldn't be present on either SoM, would it ?

>  - We avoid code duplication
> 
> This is working for us pretty well so far and the majority of the users
> of ours modules rely on this structure, we would prefer not to change that.

It may work for your current use cases, but it doesn't make it right :-)
Someone can integrate a Verdin SoM with a carrier board that has no DSI
to HDMI (or LVDS) bridge, there should thus be no such device in the
device tree. The SoM has DSI signals present on its connector, that's
what the SoM .dtsi should expose.

> [0] https://lore.kernel.org/all/20220413094449.GB118560@francesco-nb.int.toradex.com/

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-03  0:24     ` Laurent Pinchart
@ 2022-09-03 12:47       ` Francesco Dolcini
  2022-09-05 19:26         ` Laurent Pinchart
  0 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2022-09-03 12:47 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Francesco Dolcini, Max Krummenacher, Max Krummenacher,
	Fabio Estevam, Krzysztof Kozlowski, Marcel Ziswiler,
	NXP Linux Team, Pengutronix Kernel Team, Philippe Schenker,
	Rob Herring, Sascha Hauer, Shawn Guo, devicetree,
	linux-arm-kernel, linux-kernel

On Sat, Sep 03, 2022 at 03:24:51AM +0300, Laurent Pinchart wrote:
> Hi Francesco,
> 
> On Fri, Sep 02, 2022 at 05:57:20PM +0200, Francesco Dolcini wrote:
> > Hello Laurent,
> > answering here for both patches (1/2 and 2/2).
> > 
> > On Thu, Sep 01, 2022 at 09:07:49PM +0300, Laurent Pinchart wrote:
> > > On Thu, Sep 01, 2022 at 05:40:50PM +0200, Max Krummenacher wrote:
> > > > From: Max Krummenacher <max.krummenacher@toradex.com>
> > > > 
> > > > Add the hdmi connector present on the dsi to hdmi adapter now
> > > > required by the upstream lontium bridge driver.
> > > > The dsi to hdmi adapter is enabled in an device tree overlay.
> > > 
> > > Shouldn't the connector also be in the overlay ? There's certainly no
> > > physical HDMI connector on the i.MX8MP Verdin SoM :-)
> > 
> > Toradex DTS include and overlay files structure so far has been a little
> > bit different and not following the expectation you just stated here,
> > you can just check the current *toradex*dts* files and you'll see that there
> > is other stuff that is not strictly part of the module.
> > 
> > Copying from a previous email thread on a very similar discussion [0]
> > some of the reasons:
> > 
> >  - The SoM dtsi representing not only the functionality implemented into
> >    the SoM, but the whole connector pinout to the carrier makes very easy
> >    to just include a different som.dtsi in the carrier board dts and just
> >    switch SoM, for example from a colibri-imx6 to a colibri-imx7.
> 
> That's fine, but I don't see how that's related to the issue at hand.
> The DSI to HDMI bridge wouldn't be present on either SoM, would it ?
> 
> >  - We avoid code duplication
> > 
> > This is working for us pretty well so far and the majority of the users
> > of ours modules rely on this structure, we would prefer not to change that.
> 
> It may work for your current use cases, but it doesn't make it right :-)

Most of engineering is about compromise, being consistent with what we
did so far and the end-user experience need to be taken into account.

> Someone can integrate a Verdin SoM with a carrier board that has no DSI
> to HDMI (or LVDS) bridge, there should thus be no such device in the
> device tree. The SoM has DSI signals present on its connector, that's
> what the SoM .dtsi should expose.

Just for the record Verdin i.MX8M Plus do have both HDMI and LVDS on the
connector (in addition to DSI) [1], of course we do have also the option to
have LVDS or HDMI using an external add-on DSI bridge as this patches are
about.

Said that it's true that sometime we describe peripherals that are part of the
SOM family into the SOM dtsi, this avoid quite a lot of duplications given the
amount of carrier board that are available on the market that use just the same
building blocks (and this was one of the 2 points I mentioned as a reasoning
for our current DTS files structure).

Of course, we keep these stuff disabled by default, so apart for some small size
increase I do not see a real issue.

Francesco

[1] https://docs.toradex.com/110977-verdin_imx8m_plus_v1.1_datasheet.pdf

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-03 12:47       ` Francesco Dolcini
@ 2022-09-05 19:26         ` Laurent Pinchart
  2022-09-05 21:17           ` Francesco Dolcini
  0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2022-09-05 19:26 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Francesco Dolcini, Max Krummenacher, Max Krummenacher,
	Fabio Estevam, Krzysztof Kozlowski, Marcel Ziswiler,
	NXP Linux Team, Pengutronix Kernel Team, Philippe Schenker,
	Rob Herring, Sascha Hauer, Shawn Guo, devicetree,
	linux-arm-kernel, linux-kernel

Hi Francesco,

On Sat, Sep 03, 2022 at 02:47:43PM +0200, Francesco Dolcini wrote:
> On Sat, Sep 03, 2022 at 03:24:51AM +0300, Laurent Pinchart wrote:
> > On Fri, Sep 02, 2022 at 05:57:20PM +0200, Francesco Dolcini wrote:
> > > Hello Laurent,
> > > answering here for both patches (1/2 and 2/2).
> > > 
> > > On Thu, Sep 01, 2022 at 09:07:49PM +0300, Laurent Pinchart wrote:
> > > > On Thu, Sep 01, 2022 at 05:40:50PM +0200, Max Krummenacher wrote:
> > > > > From: Max Krummenacher <max.krummenacher@toradex.com>
> > > > > 
> > > > > Add the hdmi connector present on the dsi to hdmi adapter now
> > > > > required by the upstream lontium bridge driver.
> > > > > The dsi to hdmi adapter is enabled in an device tree overlay.
> > > > 
> > > > Shouldn't the connector also be in the overlay ? There's certainly no
> > > > physical HDMI connector on the i.MX8MP Verdin SoM :-)
> > > 
> > > Toradex DTS include and overlay files structure so far has been a little
> > > bit different and not following the expectation you just stated here,
> > > you can just check the current *toradex*dts* files and you'll see that there
> > > is other stuff that is not strictly part of the module.
> > > 
> > > Copying from a previous email thread on a very similar discussion [0]
> > > some of the reasons:
> > > 
> > >  - The SoM dtsi representing not only the functionality implemented into
> > >    the SoM, but the whole connector pinout to the carrier makes very easy
> > >    to just include a different som.dtsi in the carrier board dts and just
> > >    switch SoM, for example from a colibri-imx6 to a colibri-imx7.
> > 
> > That's fine, but I don't see how that's related to the issue at hand.
> > The DSI to HDMI bridge wouldn't be present on either SoM, would it ?
> > 
> > >  - We avoid code duplication
> > > 
> > > This is working for us pretty well so far and the majority of the users
> > > of ours modules rely on this structure, we would prefer not to change that.
> > 
> > It may work for your current use cases, but it doesn't make it right :-)
> 
> Most of engineering is about compromise, being consistent with what we
> did so far and the end-user experience need to be taken into account.

Sure, and so do mainline requirements :-)

> > Someone can integrate a Verdin SoM with a carrier board that has no DSI
> > to HDMI (or LVDS) bridge, there should thus be no such device in the
> > device tree. The SoM has DSI signals present on its connector, that's
> > what the SoM .dtsi should expose.
> 
> Just for the record Verdin i.MX8M Plus do have both HDMI and LVDS on the
> connector (in addition to DSI) [1], of course we do have also the option to
> have LVDS or HDMI using an external add-on DSI bridge as this patches are
> about.
> 
> Said that it's true that sometime we describe peripherals that are part of the
> SOM family into the SOM dtsi, this avoid quite a lot of duplications given the
> amount of carrier board that are available on the market that use just the same
> building blocks (and this was one of the 2 points I mentioned as a reasoning
> for our current DTS files structure).

If those "SoM family" peripherals are on the carrier board, what's the
issue with describing them in the carrier board .dtsi ? And if they're
on an add-on board (such as, if I understand correctly, the DSI to HDMI
encoder for the Dahlia carrier board), what's the issue with describing
them in an overlay ?

> Of course, we keep these stuff disabled by default, so apart for some small size
> increase I do not see a real issue.

It's the same issue as adding any DT node for peripherals that do not
exist, I fail to see a compelling reason to do so here, given that this
seems to be easy to handle in the carrier board .dtsi or in overlays.
Maybe I'm missing something ?

> [1] https://docs.toradex.com/110977-verdin_imx8m_plus_v1.1_datasheet.pdf

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-05 19:26         ` Laurent Pinchart
@ 2022-09-05 21:17           ` Francesco Dolcini
  2022-09-05 22:03             ` Laurent Pinchart
  0 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2022-09-05 21:17 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Francesco Dolcini, Francesco Dolcini, Max Krummenacher,
	Max Krummenacher, Fabio Estevam, Krzysztof Kozlowski,
	Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team,
	Philippe Schenker, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-arm-kernel, linux-kernel

Hello Laurent,

On Mon, Sep 05, 2022 at 10:26:14PM +0300, Laurent Pinchart wrote:
> On Sat, Sep 03, 2022 at 02:47:43PM +0200, Francesco Dolcini wrote:
> > On Sat, Sep 03, 2022 at 03:24:51AM +0300, Laurent Pinchart wrote:
> > > On Fri, Sep 02, 2022 at 05:57:20PM +0200, Francesco Dolcini wrote:
> > > > On Thu, Sep 01, 2022 at 09:07:49PM +0300, Laurent Pinchart wrote:
> > > Someone can integrate a Verdin SoM with a carrier board that has no DSI
> > > to HDMI (or LVDS) bridge, there should thus be no such device in the
> > > device tree. The SoM has DSI signals present on its connector, that's
> > > what the SoM .dtsi should expose.
> > 
> > Just for the record Verdin i.MX8M Plus do have both HDMI and LVDS on the
> > connector (in addition to DSI) [1], of course we do have also the option to
> > have LVDS or HDMI using an external add-on DSI bridge as this patches are
> > about.
> > 
> > Said that it's true that sometime we describe peripherals that are part of the
> > SOM family into the SOM dtsi, this avoid quite a lot of duplications given the
> > amount of carrier board that are available on the market that use just the same
> > building blocks (and this was one of the 2 points I mentioned as a reasoning
> > for our current DTS files structure).
> 
> If those "SoM family" peripherals are on the carrier board, what's the
> issue with describing them in the carrier board .dtsi ? And if they're
> on an add-on board (such as, if I understand correctly, the DSI to HDMI
> encoder for the Dahlia carrier board), what's the issue with describing
> them in an overlay ?

These SOM family peripherals are in multiples(!) carrier boards AND on
accessories. The drawback of being strict as you are asking is that we
would end-up with a massive duplication of this small DTS building
blocks, therefore the decision in the past to put those in the base SOM
dtsi file.

Maybe adding something like imx8mp-verdin-dsi-hdmi.dtsi and
imx8mp-verdin-dsi-lvds.dtsi that can be included by both overlay and
carrier dts files as needed would solve both the need of being strict on
the board definition in the dts file and avoid duplications?
Not sure if that would work smoothly, it looks like adding some
complexity and maintenance overhead, but maybe is the correct solution.

Anyway, while I fully understand your reasoning, I'm still not happy to
change this for the current toradex products, since users of
our dts file currently rely on the expectations I tried to explain in
this email thread and Max patches are implementing (and this is
currently uniform over the whole toradex product range).

> Maybe I'm missing something ?
I tried to give more insights.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality
  2022-09-05 21:17           ` Francesco Dolcini
@ 2022-09-05 22:03             ` Laurent Pinchart
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2022-09-05 22:03 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Francesco Dolcini, Max Krummenacher, Max Krummenacher,
	Fabio Estevam, Krzysztof Kozlowski, Marcel Ziswiler,
	NXP Linux Team, Pengutronix Kernel Team, Philippe Schenker,
	Rob Herring, Sascha Hauer, Shawn Guo, devicetree,
	linux-arm-kernel, linux-kernel

Hi Francesco,

On Mon, Sep 05, 2022 at 11:17:03PM +0200, Francesco Dolcini wrote:
> On Mon, Sep 05, 2022 at 10:26:14PM +0300, Laurent Pinchart wrote:
> > On Sat, Sep 03, 2022 at 02:47:43PM +0200, Francesco Dolcini wrote:
> > > On Sat, Sep 03, 2022 at 03:24:51AM +0300, Laurent Pinchart wrote:
> > > > On Fri, Sep 02, 2022 at 05:57:20PM +0200, Francesco Dolcini wrote:
> > > > > On Thu, Sep 01, 2022 at 09:07:49PM +0300, Laurent Pinchart wrote:
> > > > Someone can integrate a Verdin SoM with a carrier board that has no DSI
> > > > to HDMI (or LVDS) bridge, there should thus be no such device in the
> > > > device tree. The SoM has DSI signals present on its connector, that's
> > > > what the SoM .dtsi should expose.
> > > 
> > > Just for the record Verdin i.MX8M Plus do have both HDMI and LVDS on the
> > > connector (in addition to DSI) [1], of course we do have also the option to
> > > have LVDS or HDMI using an external add-on DSI bridge as this patches are
> > > about.
> > > 
> > > Said that it's true that sometime we describe peripherals that are part of the
> > > SOM family into the SOM dtsi, this avoid quite a lot of duplications given the
> > > amount of carrier board that are available on the market that use just the same
> > > building blocks (and this was one of the 2 points I mentioned as a reasoning
> > > for our current DTS files structure).
> > 
> > If those "SoM family" peripherals are on the carrier board, what's the
> > issue with describing them in the carrier board .dtsi ? And if they're
> > on an add-on board (such as, if I understand correctly, the DSI to HDMI
> > encoder for the Dahlia carrier board), what's the issue with describing
> > them in an overlay ?
> 
> These SOM family peripherals are in multiples(!) carrier boards AND on
> accessories. The drawback of being strict as you are asking is that we
> would end-up with a massive duplication of this small DTS building
> blocks, therefore the decision in the past to put those in the base SOM
> dtsi file.

OK, I got it now.

> Maybe adding something like imx8mp-verdin-dsi-hdmi.dtsi and
> imx8mp-verdin-dsi-lvds.dtsi that can be included by both overlay and
> carrier dts files as needed would solve both the need of being strict on
> the board definition in the dts file and avoid duplications?
> Not sure if that would work smoothly, it looks like adding some
> complexity and maintenance overhead, but maybe is the correct solution.

That sounds good to me. Would you be able to give it a try to see if it
works well ?

> Anyway, while I fully understand your reasoning, I'm still not happy to
> change this for the current toradex products, since users of
> our dts file currently rely on the expectations I tried to explain in
> this email thread and Max patches are implementing (and this is
> currently uniform over the whole toradex product range).

This sounds like a broader question, not specific to Toradex, opinions
from Rob and Krzysztof would be useful.

> > Maybe I'm missing something ?
> 
> I tried to give more insights.

Thank you, that's very appreciated.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-09-05 22:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 15:40 [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Max Krummenacher
2022-09-01 15:40 ` [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality Max Krummenacher
2022-09-01 18:10   ` Laurent Pinchart
2022-09-01 18:07 ` [PATCH 1/2] arm64: dts: imx8mp-verdin: add dsi to hdmi functionality Laurent Pinchart
2022-09-02 15:57   ` Francesco Dolcini
2022-09-03  0:24     ` Laurent Pinchart
2022-09-03 12:47       ` Francesco Dolcini
2022-09-05 19:26         ` Laurent Pinchart
2022-09-05 21:17           ` Francesco Dolcini
2022-09-05 22:03             ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).