* [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support
@ 2024-07-26 6:50 Liu Ying
2024-07-27 11:15 ` Dmitry Baryshkov
0 siblings, 1 reply; 5+ messages in thread
From: Liu Ying @ 2024-07-26 6:50 UTC (permalink / raw)
To: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel
Cc: robh, krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam, saravanak
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@nxp.com>
---
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.
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 {
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 {
+ compatible = "hannstar,hsd100pxn1";
+ backlight = <&backlight>;
+ power-supply = <®_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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support
2024-07-26 6:50 [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support Liu Ying
@ 2024-07-27 11:15 ` Dmitry Baryshkov
2024-07-29 4:47 ` Liu Ying
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Baryshkov @ 2024-07-27 11:15 UTC (permalink / raw)
To: Liu Ying
Cc: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel, robh,
krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
saravanak
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@nxp.com>
> ---
> 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 = <®_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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support
2024-07-27 11:15 ` Dmitry Baryshkov
@ 2024-07-29 4:47 ` Liu Ying
2024-07-30 2:48 ` Liu Ying
0 siblings, 1 reply; 5+ messages in thread
From: Liu Ying @ 2024-07-29 4:47 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel, robh,
krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
saravanak
Hi Dmitry,
On 07/27/2024, Dmitry Baryshkov wrote:
> 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@nxp.com>
>> ---
>> 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?
I fdtdump imx53-qsrb-mcimx-lvds1.dtb and imx53-qsrb.dtb.
I see three sensible differences.
1) panel-lvds node position.
For imx53-qsrb-mcimx-lvds1.dtb, it comes very early and is next to
'compatible = "fsl,imx53-qsrb", "fsl,imx53";'.
For imx53-qsrb.dtb, it comes later and is next to panel node in '/' node.
2) properties order in panel-lvds node.
For imx53-qsrb-mcimx-lvds1.dtb, it shows
panel-lvds {
power-supply = <0x0000001c>;
backlight = <0x00000030>;
compatible = "hannstar,hsd100pxn1";
port {
endpoint {
phandle = <0x0000007d>;
remote-endpoint = <0x0000007c>;
};
};
};
For imx53-qsrb.dtb, it shows
panel-lvds {
compatible = "hannstar,hsd100pxn1";
backlight = <0x00000031>;
power-supply = <0x0000001d>;
port {
endpoint {
remote-endpoint = <0x00000033>;
phandle = <0x00000017>;
};
};
};
3) No 'lvds0_out' and 'panel_lvds_in' in __symbols__ node for
imx53-qsrb-mcimx-lvds1.dtb, but for imx53-qsrb.dtb they are in it.
lvds0_out = "/soc/bus@50000000/ldb@53fa8008/lvds-channel@0/port@2/endpoint";
panel_lvds_in = "/panel-lvds/port/endpoint";
BTW, reverting Saravana's commits
7cb50f6c9fba ("of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing")
and/or
7fddac12c382 ("driver core: Fix device_link_flag_is_sync_state_only()")
avoids the issue from happening.
>
>>
>> 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
Do you suggest to do this in a separate patch?
If yes, is it worth adding a Fixes tag?
>
>> 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.
Nope.
'panel-lvds' is needed to differentiate it from 'panel' in
imx53-qsb-common.dtsi which is a DPI panel.
Using 'panel-lvds', procfs lists exactly the properties needed.
root@imx53qsb:~# ls /proc/device-tree/panel-lvds/
backlight compatible name port power-supply
Using 'panel', more are listed.
root@imx53qsb:~# ls /proc/device-tree/panel/
backlight compatible enable-gpios name phandle pinctrl-0 pinctrl-names port power-supply
>
>> + compatible = "hannstar,hsd100pxn1";
>> + backlight = <&backlight>;
>> + power-supply = <®_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
>>
>
--
Regards,
Liu Ying
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support
2024-07-29 4:47 ` Liu Ying
@ 2024-07-30 2:48 ` Liu Ying
2024-08-13 9:31 ` Liu Ying
0 siblings, 1 reply; 5+ messages in thread
From: Liu Ying @ 2024-07-30 2:48 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel, robh,
krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
saravanak
On 07/29/2024, Liu Ying wrote:
> Hi Dmitry,
>
> On 07/27/2024, Dmitry Baryshkov wrote:
>> 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@nxp.com>
>>> ---
>>> 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?
>
> I fdtdump imx53-qsrb-mcimx-lvds1.dtb and imx53-qsrb.dtb.
>
> I see three sensible differences.
> 1) panel-lvds node position.
> For imx53-qsrb-mcimx-lvds1.dtb, it comes very early and is next to
> 'compatible = "fsl,imx53-qsrb", "fsl,imx53";'.
> For imx53-qsrb.dtb, it comes later and is next to panel node in '/' node.
It turns out only 1) panel-lvds node position matters.
I can reproduce the issue with imx53-qsrb.dtb(no DT overlay) if I put
the panel-lvds node before the soc node. If the panel-lvds node is
after the soc node, then the issue doesn't happen with imx53-qsrb.dtb.
The ldb node(LVDS display bridge) and IPU(display controller) node are
in the soc node. Maybe, the order of the ldb node and the panel-lvds
node in DT blob matters(be my guess).
>
> 2) properties order in panel-lvds node.
> For imx53-qsrb-mcimx-lvds1.dtb, it shows
> panel-lvds {
> power-supply = <0x0000001c>;
> backlight = <0x00000030>;
> compatible = "hannstar,hsd100pxn1";
> port {
> endpoint {
> phandle = <0x0000007d>;
> remote-endpoint = <0x0000007c>;
> };
> };
> };
> For imx53-qsrb.dtb, it shows
> panel-lvds {
> compatible = "hannstar,hsd100pxn1";
> backlight = <0x00000031>;
> power-supply = <0x0000001d>;
> port {
> endpoint {
> remote-endpoint = <0x00000033>;
> phandle = <0x00000017>;
> };
> };
> };
>
> 3) No 'lvds0_out' and 'panel_lvds_in' in __symbols__ node for
> imx53-qsrb-mcimx-lvds1.dtb, but for imx53-qsrb.dtb they are in it.
> lvds0_out = "/soc/bus@50000000/ldb@53fa8008/lvds-channel@0/port@2/endpoint";
> panel_lvds_in = "/panel-lvds/port/endpoint";
>
> BTW, reverting Saravana's commits
> 7cb50f6c9fba ("of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing")
> and/or
> 7fddac12c382 ("driver core: Fix device_link_flag_is_sync_state_only()")
> avoids the issue from happening.
>
>>
>>>
>>> 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
>
> Do you suggest to do this in a separate patch?
> If yes, is it worth adding a Fixes tag?
>
>>
>>> 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.
>
> Nope.
>
> 'panel-lvds' is needed to differentiate it from 'panel' in
> imx53-qsb-common.dtsi which is a DPI panel.
>
> Using 'panel-lvds', procfs lists exactly the properties needed.
> root@imx53qsb:~# ls /proc/device-tree/panel-lvds/
> backlight compatible name port power-supply
>
> Using 'panel', more are listed.
> root@imx53qsb:~# ls /proc/device-tree/panel/
> backlight compatible enable-gpios name phandle pinctrl-0 pinctrl-names port power-supply
>
>>
>>> + compatible = "hannstar,hsd100pxn1";
>>> + backlight = <&backlight>;
>>> + power-supply = <®_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
>>>
>>
>
--
Regards,
Liu Ying
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support
2024-07-30 2:48 ` Liu Ying
@ 2024-08-13 9:31 ` Liu Ying
0 siblings, 0 replies; 5+ messages in thread
From: Liu Ying @ 2024-08-13 9:31 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: devicetree, imx, linux-arm-kernel, linux-kernel, dri-devel, robh,
krzk+dt, conor+dt, shawnguo, s.hauer, kernel, festevam,
saravanak
On 07/30/2024, Liu Ying wrote:
> On 07/29/2024, Liu Ying wrote:
>> Hi Dmitry,
>>
>> On 07/27/2024, Dmitry Baryshkov wrote:
>>> 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@nxp.com>
>>>> ---
>>>> 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?
>>
>> I fdtdump imx53-qsrb-mcimx-lvds1.dtb and imx53-qsrb.dtb.
>>
>> I see three sensible differences.
>> 1) panel-lvds node position.
>> For imx53-qsrb-mcimx-lvds1.dtb, it comes very early and is next to
>> 'compatible = "fsl,imx53-qsrb", "fsl,imx53";'.
>> For imx53-qsrb.dtb, it comes later and is next to panel node in '/' node.
>
> It turns out only 1) panel-lvds node position matters.
Hi Saravana,
It looks like this issue is caused by/related to fw_devlink.
Any thoughts please?
>
> I can reproduce the issue with imx53-qsrb.dtb(no DT overlay) if I put
> the panel-lvds node before the soc node. If the panel-lvds node is
> after the soc node, then the issue doesn't happen with imx53-qsrb.dtb.
>
> The ldb node(LVDS display bridge) and IPU(display controller) node are
> in the soc node. Maybe, the order of the ldb node and the panel-lvds
> node in DT blob matters(be my guess).
>
>>
>> 2) properties order in panel-lvds node.
>> For imx53-qsrb-mcimx-lvds1.dtb, it shows
>> panel-lvds {
>> power-supply = <0x0000001c>;
>> backlight = <0x00000030>;
>> compatible = "hannstar,hsd100pxn1";
>> port {
>> endpoint {
>> phandle = <0x0000007d>;
>> remote-endpoint = <0x0000007c>;
>> };
>> };
>> };
>> For imx53-qsrb.dtb, it shows
>> panel-lvds {
>> compatible = "hannstar,hsd100pxn1";
>> backlight = <0x00000031>;
>> power-supply = <0x0000001d>;
>> port {
>> endpoint {
>> remote-endpoint = <0x00000033>;
>> phandle = <0x00000017>;
>> };
>> };
>> };
>>
>> 3) No 'lvds0_out' and 'panel_lvds_in' in __symbols__ node for
>> imx53-qsrb-mcimx-lvds1.dtb, but for imx53-qsrb.dtb they are in it.
>> lvds0_out = "/soc/bus@50000000/ldb@53fa8008/lvds-channel@0/port@2/endpoint";
>> panel_lvds_in = "/panel-lvds/port/endpoint";
>>
>> BTW, reverting Saravana's commits
>> 7cb50f6c9fba ("of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing")
>> and/or
>> 7fddac12c382 ("driver core: Fix device_link_flag_is_sync_state_only()")
>> avoids the issue from happening.
>>
>>>
>>>>
>>>> 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
>>
>> Do you suggest to do this in a separate patch?
>> If yes, is it worth adding a Fixes tag?
>>
>>>
>>>> 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.
>>
>> Nope.
>>
>> 'panel-lvds' is needed to differentiate it from 'panel' in
>> imx53-qsb-common.dtsi which is a DPI panel.
>>
>> Using 'panel-lvds', procfs lists exactly the properties needed.
>> root@imx53qsb:~# ls /proc/device-tree/panel-lvds/
>> backlight compatible name port power-supply
>>
>> Using 'panel', more are listed.
>> root@imx53qsb:~# ls /proc/device-tree/panel/
>> backlight compatible enable-gpios name phandle pinctrl-0 pinctrl-names port power-supply
>>
>>>
>>>> + compatible = "hannstar,hsd100pxn1";
>>>> + backlight = <&backlight>;
>>>> + power-supply = <®_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
>>>>
>>>
>>
>
--
Regards,
Liu Ying
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-08-13 9:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-26 6:50 [RFC PATCH] ARM: dts: imx53-qsb: Add MCIMX-LVDS1 display module support Liu Ying
2024-07-27 11:15 ` Dmitry Baryshkov
2024-07-29 4:47 ` Liu Ying
2024-07-30 2:48 ` Liu Ying
2024-08-13 9:31 ` Liu Ying
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®