Re: [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support

[Dmitry Baryshkov]

On Fri, Jul 26, 2024 at 02:50:12PM GMT, Liu Ying wrote:
> MCIMX-LVDS1[1] display module integrates a HannStar HSD100PXN1 LVDS
> display panel and a touch IC.  Add an overlay to support the LVDS
> panel on i.MX53 QSB / QSRB platforms.
> 
> [1] https://www.nxp.com/part/MCIMX-LVDS1
> 
> Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> ---
> I mark RFC in patch subject prefix because if the DT overlay is used, both ldb
> and panel devices end up as devices deferred.  However, if the DT overlay is
> not used and the devices are defined in imx53-qsb-common.dtsi, then they can be
> probed ok.
> 
> With a dev_err_probe() added to imx_ldb_probe() in imx-ldb.c, devices_deferred
> indicates 53fa8008.ldb and panel-lvds kind of depend on each other.
> 
> root@imx53qsb:~# cat /sys/kernel/debug/devices_deferred
> 53fa8008.ldb    imx-ldb: failed to find panel or bridge for channel0
> panel-lvds      platform: wait for supplier /soc/bus@50000000/ldb@53fa8008/lvds-channel@0
> 
> It looks like the issue is related to fw_devlink, because if "fw_devlink=off"
> is added to kernel bootup command line, then the issue doesn't happen.

Could you please fdtdump /sys/firmware/fdt (or just generated DTB files)
in both cases and compare the dumps for sensible differences?

> 
> Saravana, DT folks, any ideas?
> 
> Thanks.
> 
>  arch/arm/boot/dts/nxp/imx/Makefile            |  4 ++
>  .../boot/dts/nxp/imx/imx53-qsb-common.dtsi    |  4 +-
>  .../dts/nxp/imx/imx53-qsb-mcimx-lvds1.dtso    | 43 +++++++++++++++++++
>  3 files changed, 49 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/boot/dts/nxp/imx/imx53-qsb-mcimx-lvds1.dtso
> 
> diff --git a/arch/arm/boot/dts/nxp/imx/Makefile b/arch/arm/boot/dts/nxp/imx/Makefile
> index 92e291603ea1..7116889e1515 100644
> --- a/arch/arm/boot/dts/nxp/imx/Makefile
> +++ b/arch/arm/boot/dts/nxp/imx/Makefile
> @@ -46,8 +46,10 @@ dtb-$(CONFIG_SOC_IMX53) += \
>  	imx53-ppd.dtb \
>  	imx53-qsb.dtb \
>  	imx53-qsb-hdmi.dtb \
> +	imx53-qsb-mcimx-lvds1.dtb \
>  	imx53-qsrb.dtb \
>  	imx53-qsrb-hdmi.dtb \
> +	imx53-qsrb-mcimx-lvds1.dtb \
>  	imx53-sk-imx53.dtb \
>  	imx53-sk-imx53-atm0700d4-lvds.dtb \
>  	imx53-sk-imx53-atm0700d4-rgb.dtb \
> @@ -57,7 +59,9 @@ dtb-$(CONFIG_SOC_IMX53) += \
>  	imx53-usbarmory.dtb \
>  	imx53-voipac-bsb.dtb
>  imx53-qsb-hdmi-dtbs := imx53-qsb.dtb imx53-qsb-hdmi.dtbo
> +imx53-qsb-mcimx-lvds1-dtbs := imx53-qsb.dtb imx53-qsb-mcimx-lvds1.dtbo
>  imx53-qsrb-hdmi-dtbs := imx53-qsrb.dtb imx53-qsb-hdmi.dtbo
> +imx53-qsrb-mcimx-lvds1-dtbs := imx53-qsrb.dtb imx53-qsb-mcimx-lvds1.dtbo
>  dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6dl-alti6p.dtb \
>  	imx6dl-apf6dev.dtb \
> diff --git a/arch/arm/boot/dts/nxp/imx/imx53-qsb-common.dtsi b/arch/arm/boot/dts/nxp/imx/imx53-qsb-common.dtsi
> index 05d7a462ea25..430792a91ccf 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx53-qsb-common.dtsi
> +++ b/arch/arm/boot/dts/nxp/imx/imx53-qsb-common.dtsi
> @@ -16,7 +16,7 @@ memory@70000000 {
>  		      <0xb0000000 0x20000000>;
>  	};
>  
> -	backlight_parallel: backlight-parallel {
> +	backlight: backlight {

Nit: this seems unrelated to the LVDS support

>  		compatible = "pwm-backlight";
>  		pwms = <&pwm2 0 5000000 0>;
>  		brightness-levels = <0 4 8 16 32 64 128 255>;
> @@ -89,7 +89,7 @@ panel_dpi: panel {
>  		compatible = "sii,43wvf1g";
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&pinctrl_display_power>;
> -		backlight = <&backlight_parallel>;
> +		backlight = <&backlight>;
>  		enable-gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>;
>  
>  		port {
> diff --git a/arch/arm/boot/dts/nxp/imx/imx53-qsb-mcimx-lvds1.dtso b/arch/arm/boot/dts/nxp/imx/imx53-qsb-mcimx-lvds1.dtso
> new file mode 100644
> index 000000000000..27f6bedf3d39
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx53-qsb-mcimx-lvds1.dtso
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&{/} {
> +	panel-lvds {

Nit: Just 'panel' should be enough.

> +		compatible = "hannstar,hsd100pxn1";
> +		backlight = <&backlight>;
> +		power-supply = <&reg_3p2v>;
> +
> +		port {
> +			panel_lvds_in: endpoint {
> +				remote-endpoint = <&lvds0_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&ldb {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	lvds-channel@0 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		fsl,data-mapping = "spwg";
> +		fsl,data-width = <18>;
> +		status = "okay";
> +
> +		port@2 {
> +			reg = <2>;
> +
> +			lvds0_out: endpoint {
> +				remote-endpoint = <&panel_lvds_in>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry