[PATCH 0/8] Add display support for STM32MP25

[PATCH 0/8] Add display support for STM32MP25

[Raphael Gallais-Pou]

This series aims to add basic LVDS display support for STM32MP257F-EV1
board.

It introduces:
- the STM32 LVDS Display Interface Transmitter driver.
- a new version of the STM32 LTDC driver.
- the support of "panel-lvds" compatible.

It adds and enables driver support for stm32mp257f-ev1-u-boot
It also brings fixes to the ofnode driver.

This work is based on the Linux device-tree sent recently [1].

[1] https://lore.kernel.org/lkml/20250819-drm-misc-next-v3-0-04153978ebdb@foss.st.com

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
Raphael Gallais-Pou (8):
      ofnode: support panel-timings in ofnode_decode_display_timing
      video: simple_panel: add support for "panel-lvds" display
      video: stm32: STM32 driver support for LVDS
      video: stm32: ltdc: support new hardware version for STM32MP25 SoC
      video: stm32: ltdc: properly search the first available panel
      ARM: dts: stm32: use LTDC and LVDS nodes before relocation in stm32mp25-u-boot
      ARM: dts: stm32: remove panel property in stm32mp257f-ev1-u-boot
      configs: stm32mp25: enable LVDS display support

 MAINTAINERS                              |   1 +
 arch/arm/dts/stm32mp25-u-boot.dtsi       |   8 +
 arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi |   4 +
 configs/stm32mp25_defconfig              |   3 +
 doc/board/st/st-dt.rst                   |   1 +
 drivers/core/ofnode.c                    |  17 +-
 drivers/video/simple_panel.c             |   1 +
 drivers/video/stm32/Kconfig              |   9 +
 drivers/video/stm32/Makefile             |   1 +
 drivers/video/stm32/stm32_ltdc.c         | 157 ++++++-
 drivers/video/stm32/stm32_lvds.c         | 673 +++++++++++++++++++++++++++++++
 11 files changed, 855 insertions(+), 20 deletions(-)
---
base-commit: 54fbdd408873413af86d86de03a490c5abf280f1
change-id: 20250813-master-23ce98cadee0

Best regards,
-- 
Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>

[PATCH 1/8] ofnode: support panel-timings in ofnode_decode_display_timing

[Raphael Gallais-Pou]

The "Display Timings" in panel-common.yaml can be provided by 2 properties
- panel-timing: when display panels are restricted to a single resolution
                the "panel-timing" node expresses the required timings.
- display-timings: several resolutions with different timings are supported
                   with several timing subnode of "display-timings" node

This patch update the parsing function to handle this 2 possibility
when index = 0.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/core/ofnode.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index e040e3f2806ffe74c58dcd82f36307351acd5a99..5a721b46e5a3214e7bd437739776362c2d22a3c9 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1221,13 +1221,16 @@ int ofnode_decode_display_timing(ofnode parent, int index,
 	int ret = 0;
 
 	timings = ofnode_find_subnode(parent, "display-timings");
-	if (!ofnode_valid(timings))
-		return -EINVAL;
-
-	i = 0;
-	ofnode_for_each_subnode(node, timings) {
-		if (i++ == index)
-			break;
+	if (ofnode_valid(timings)) {
+		i = 0;
+		ofnode_for_each_subnode(node, timings) {
+			if (i++ == index)
+				break;
+		}
+	} else {
+		if (index != 0)
+			return -EINVAL;
+		node = ofnode_find_subnode(parent, "panel-timing");
 	}
 
 	if (!ofnode_valid(node))

-- 
2.25.1

[PATCH 2/8] video: simple_panel: add support for "panel-lvds" display

[Raphael Gallais-Pou]

Add the compatible "panel-lvds" for simple-panel driver in U-Boot.  In
Linux this compatible is managed by the driver
drivers/gpu/drm/panel/panel-lvds.c but in U-Boot the specific LVDS
features (bus_format/bus_flags) are not supported.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/video/simple_panel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/simple_panel.c b/drivers/video/simple_panel.c
index b6c5b058b2e967bbbd47ab3c3ce5ca52c7804409..0f23df701bc3c40ea49380bbfa3743ee592d8bd3 100644
--- a/drivers/video/simple_panel.c
+++ b/drivers/video/simple_panel.c
@@ -191,6 +191,7 @@ static const struct mipi_dsi_panel_plat panasonic_vvx10f004b00 = {
 
 static const struct udevice_id simple_panel_ids[] = {
 	{ .compatible = "simple-panel" },
+	{ .compatible = "panel-lvds" },
 	{ .compatible = "auo,b133xtn01" },
 	{ .compatible = "auo,b116xw03" },
 	{ .compatible = "auo,b133htn01" },

-- 
2.25.1

[PATCH 3/8] video: stm32: STM32 driver support for LVDS

[Raphael Gallais-Pou]

The LVDS Display Interface Transmitter handles the LVDS protocol:
it maps the pixels received from the upstream Pixel-DMA (LTDC)
onto the LVDS PHY.

The LVDS controller driver supports the following high-level features:
        • FDP-Link-I and OpenLDI (v0.95) protocols
        • Single-Link or Dual-Link operation
        • Single-Display or Double-Display (with the same content
          duplicated on both)
        • Flexible Bit-Mapping, including JEIDA and VESA
        • RGB888 or RGB666 output
        • Synchronous design, with one input pixel per clock cycle
        • No resolution limitation.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 MAINTAINERS                      |   1 +
 doc/board/st/st-dt.rst           |   1 +
 drivers/video/stm32/Kconfig      |   9 +
 drivers/video/stm32/Makefile     |   1 +
 drivers/video/stm32/stm32_lvds.c | 673 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 685 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3fb163aa1db1dba249e60b40ab3e785db4fde5d3..604ba4ca04c1079b14cd8f4c1fff4c12da0e278d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -728,6 +728,7 @@ F:	drivers/serial/serial_stm32.*
 F:	drivers/spi/stm32_qspi.c
 F:	drivers/spi/stm32_spi.c
 F:	drivers/video/stm32/stm32_ltdc.c
+F:	drivers/video/stm32/stm32_lvds.c
 F:	drivers/watchdog/stm32mp_wdt.c
 F:	include/dt-bindings/clock/stm32fx-clock.h
 F:	include/dt-bindings/clock/stm32mp*
diff --git a/doc/board/st/st-dt.rst b/doc/board/st/st-dt.rst
index 2a285c81807ee26924948d8c2497f11dfe45b6e6..28cada97e72da711408a1aaaa928ad3faa51162e 100644
--- a/doc/board/st/st-dt.rst
+++ b/doc/board/st/st-dt.rst
@@ -25,6 +25,7 @@ kernel binding directory = Documentation/devicetree/bindings/
 * display
 	- display/st,stm32-dsi.yaml
 	- display/st,stm32-ltdc.yaml
+        - display/st,stm32mp25-lvds.yaml
 * gpio
 	- pinctrl/st,stm32-pinctrl.yaml
 * hwlock
diff --git a/drivers/video/stm32/Kconfig b/drivers/video/stm32/Kconfig
index c354c402c288b4c004bd2b5cc74a4d00beef2773..4cb8a841caf51aaba5115b9fcddb967f73a3d9b4 100644
--- a/drivers/video/stm32/Kconfig
+++ b/drivers/video/stm32/Kconfig
@@ -23,6 +23,15 @@ config VIDEO_STM32_DSI
 	  This option enables support DSI internal bridge which can be used on
 	  devices which have DSI devices connected.
 
+config VIDEO_STM32_LVDS
+	bool "Enable STM32 LVDS video support"
+	depends on VIDEO_STM32
+	select VIDEO_BRIDGE
+	select VIDEO_DW_MIPI_DSI
+	help
+	  This enables Low Voltage Differential Signaling (LVDS) display
+	  support.
+
 config VIDEO_STM32_MAX_XRES
 	int "Maximum horizontal resolution (for memory allocation purposes)"
 	depends on VIDEO_STM32
diff --git a/drivers/video/stm32/Makefile b/drivers/video/stm32/Makefile
index f8b42d1a4d126a31aa11ce820e829b133ca3bb2d..059d9000c1d7762d06bff36d5408143a34848227 100644
--- a/drivers/video/stm32/Makefile
+++ b/drivers/video/stm32/Makefile
@@ -7,3 +7,4 @@
 
 obj-${CONFIG_VIDEO_STM32} = stm32_ltdc.o
 obj-${CONFIG_VIDEO_STM32_DSI} += stm32_dsi.o
+obj-${CONFIG_VIDEO_STM32_LVDS} += stm32_lvds.o
diff --git a/drivers/video/stm32/stm32_lvds.c b/drivers/video/stm32/stm32_lvds.c
new file mode 100644
index 0000000000000000000000000000000000000000..1e3e740122dfee0c947819532da64f0d99327bde
--- /dev/null
+++ b/drivers/video/stm32/stm32_lvds.c
@@ -0,0 +1,673 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
+ * Author(s): Raphaël Gallais-Pou <raphael.gallais-pou@foss.st.com> for STMicroelectronics.
+ *
+ * This Low Voltage Differential Signal controller driver is based on the Linux Kernel driver from
+ * drivers/gpu/drm/stm/ltdc.c
+ */
+
+#define LOG_CATEGORY UCLASS_VIDEO_BRIDGE
+
+#include <clk.h>
+#include <dm.h>
+#include <log.h>
+#include <media_bus_format.h>
+#include <panel.h>
+#include <reset.h>
+#include <video.h>
+#include <video_bridge.h>
+#include <asm/io.h>
+#include <dm/device_compat.h>
+#include <dm/ofnode.h>
+#include <linux/iopoll.h>
+
+/* LVDS Host registers */
+#define LVDS_CR		0x0000  /* configuration register */
+#define LVDS_DMLCR0	0x0004  /* data mapping lsb configuration register 0	*/
+#define LVDS_DMMCR0	0x0008  /* data mapping msb configuration register 0	*/
+#define LVDS_DMLCR1	0x000C  /* data mapping lsb configuration register 1	*/
+#define LVDS_DMMCR1	0x0010  /* data mapping msb configuration register 1	*/
+#define LVDS_DMLCR2	0x0014  /* data mapping lsb configuration register 2	*/
+#define LVDS_DMMCR2	0x0018  /* data mapping msb configuration register 2	*/
+#define LVDS_DMLCR3	0x001C  /* data mapping lsb configuration register 3	*/
+#define LVDS_DMMCR3	0x0020  /* data mapping msb configuration register 3	*/
+#define LVDS_DMLCR4	0x0024  /* data mapping lsb configuration register 4	*/
+#define LVDS_DMMCR4	0x0028  /* data mapping msb configuration register 4	*/
+#define LVDS_DMLCR(id)	(LVDS_DMLCR0 + 8U * (id))
+#define LVDS_DMMCR(id)	(LVDS_DMMCR0 + 8U * (id))
+#define LVDS_CDL1CR	0x002C  /* channel distrib link 1 configuration register	*/
+#define LVDS_CDL2CR	0x0030  /* channel distrib link 2 configuration register	*/
+
+#define CDL1CR_DEFAULT	0x4321
+#define CDL2CR_DEFAULT	0x59876
+
+/* LVDS Host registers */
+#define LVDS_PHY_MASTER	0x0
+#define LVDS_PHY_SLAVE	0x100
+
+/* phy parameter can only be one of those two above */
+#define LVDS_PxGCR(phy)		((phy) + 0x1000)   /* Global Control Register	*/
+#define LVDS_PxCMCR1(phy)	((phy) + 0x100C)   /* Current Mode Control Register 1 */
+#define LVDS_PxCMCR2(phy)	((phy) + 0x1010)  /* Current Mode Control Register 2 */
+#define LVDS_PxSCR(phy)		((phy) + 0x1020)  /* Serial Control Register	*/
+#define LVDS_PxBCR1(phy)	((phy) + 0x102C)  /* Bias Control Register 1	*/
+#define LVDS_PxBCR2(phy)	((phy) + 0x1030)  /* Bias Control Register 2	*/
+#define LVDS_PxBCR3(phy)	((phy) + 0x1034)  /* Bias Control Register 3	*/
+#define LVDS_PxMPLCR(phy)	((phy) + 0x1064)  /* Monitor PLL Lock Control Register */
+#define LVDS_PxDCR(phy)		((phy) + 0x1084)  /* Debug Control Register	*/
+#define LVDS_PxSSR1(phy)	((phy) + 0x1088)  /* Spare Status Register 1	*/
+#define LVDS_PxCFGCR(phy)	((phy) + 0x10A0)  /* Configuration Control Register */
+#define LVDS_PxPLLCR1(phy)	((phy) + 0x10C0)  /* PLL_MODE 1 Control Register	*/
+#define LVDS_PxPLLCR2(phy)	((phy) + 0x10C4)  /* PLL_MODE 2 Control Register	*/
+#define LVDS_PxPLLSR(phy)	((phy) + 0x10C8)  /* PLL Status Register	*/
+#define LVDS_PxPLLSDCR1(phy)	((phy) + 0x10CC)  /* PLL_SD_1 Control Register	*/
+#define LVDS_PxPLLSDCR2(phy)	((phy) + 0x10D0)  /* PLL_SD_2 Control Register	*/
+#define LVDS_PxPLLTWGCR1(phy)	((phy) + 0x10D4)  /* PLL_TWG_1 Control Register	*/
+#define LVDS_PxPLLTWGCR2(phy)	((phy) + 0x10D8)  /* PLL_TWG_2 Control Register	*/
+#define LVDS_PxPLLCPCR(phy)	((phy) + 0x10E0)  /* PLL_CP Control Register	*/
+#define LVDS_PxPLLTESTCR(phy)	((phy) + 0x10E8)  /* PLL_TEST Control Register	*/
+
+/* LVDS Wrapper registers */
+#define LVDS_WCLKCR	0x11B0  /* Wrapper clock control register */
+#define LVDS_HWCFGR	0x1FF0  /* HW configuration register	*/
+#define LVDS_VERR	0x1FF4  /* Version register	*/
+#define LVDS_IPIDR	0x1FF8  /* Identification register	*/
+#define LVDS_SIDR	0x1FFC  /* Size Identification register	*/
+
+#define CR_LVDSEN	BIT(0)  /* LVDS PHY Enable */
+#define CR_HSPOL	BIT(1)  /* HS Polarity (horizontal sync) */
+#define CR_VSPOL	BIT(2)  /* VS Polarity (vertical sync) */
+#define CR_DEPOL	BIT(3)  /* DE Polarity (data enable) */
+#define CR_CI		BIT(4)  /* Control Internal (software controlled bit) */
+#define CR_LKMOD	BIT(5)  /* Link Mode, for both Links */
+#define CR_LKPHA	BIT(6)  /* Link Phase, for both Links */
+#define CR_LK1POL	GENMASK(20, 16)  /* Link-1 output Polarity */
+#define CR_LK2POL	GENMASK(25, 21)  /* Link-2 output Polarity */
+
+#define DMMCRx_MAP0	GENMASK(4, 0)
+#define DMMCRx_MAP1	GENMASK(9, 5)
+#define DMMCRx_MAP2	GENMASK(14, 10)
+#define DMMCRx_MAP3	GENMASK(19, 15)
+#define DMLCRx_MAP4	GENMASK(4, 0)
+#define DMLCRx_MAP5	GENMASK(9, 5)
+#define DMLCRx_MAP6	GENMASK(14, 10)
+
+#define CDLCRx_DISTR0	GENMASK(3, 0)
+#define CDLCRx_DISTR1	GENMASK(7, 4)
+#define CDLCRx_DISTR2	GENMASK(11, 8)
+#define CDLCRx_DISTR3	GENMASK(15, 12)
+#define CDLCRx_DISTR4	GENMASK(19, 16)
+
+#define FREF_INDEX	0
+#define NDIV_INDEX	1
+#define FPFD_INDEX	2
+#define MDIV_INDEX	3
+#define FVCO_INDEX	4
+#define BDIV_INDEX	5
+#define FBIT_INDEX	6
+#define FLS_INDEX	7
+#define FDP_INDEX	8
+
+#define PHY_GCR_BIT_CLK_OUT	BIT(0)
+#define PHY_GCR_LS_CLK_OUT	BIT(4)
+#define PHY_GCR_DP_CLK_OUT	BIT(8)
+#define PHY_GCR_RSTZ		BIT(24)
+#define PHY_GCR_DIV_RSTN	BIT(25)
+
+#define PHY_PxPLLTESTCR_TDIV	GENMASK(25, 16)
+#define PHY_PxPLLCR2_NDIV	GENMASK(25, 16)
+#define PHY_PxPLLCR2_BDIV	GENMASK(9, 0)
+#define PHY_PxPLLSDCR1_MDIV	GENMASK(9, 0)
+
+#define PLL_EN		BIT(0)
+#define PLL_LOCK	BIT(0)
+#define CM_EN_DL	(BIT(28) | BIT(20) | BIT(12) | BIT(4))
+#define CM_EN_DL4	BIT(4)
+#define VM_EN_DL	(BIT(16) | BIT(12) | BIT(8) | BIT(4) | BIT(0))
+#define EN_BIAS_DL	(BIT(16) | BIT(12) | BIT(8) | BIT(4) | BIT(0))
+#define EN_DIG_DL	GENMASK(4, 0)
+#define BIAS_EN		BIT(28)
+#define POWER_OK	BIT(12)
+
+#define WCLKCR_SLV_CLKPIX_SEL	BIT(0)
+#define WCLKCR_SRCSEL		BIT(8)
+
+/* Sleep & timeout for pll lock/unlock */
+#define SLEEP_US	1000
+#define TIMEOUT_US	20000000
+
+#define PHY_SLV_OFS	0x100
+
+struct stm32_lvds {
+	void __iomem *base;
+	struct udevice *panel;
+	u32 refclk;
+	int dual_link;
+	int bus_format;
+};
+
+/*
+ * enum lvds_pixels_order - Pixel order of an LVDS connection
+ * @LVDS_DUAL_LINK_EVEN_ODD_PIXELS: Even pixels are expected to be generated
+ *    from the first port, odd pixels from the second port
+ * @LVDS_DUAL_LINK_ODD_EVEN_PIXELS: Odd pixels are expected to be generated
+ *    from the first port, even pixels from the second port
+ */
+enum lvds_pixels_order {
+	LVDS_DUAL_LINK_EVEN_ODD_PIXELS = BIT(0),
+	LVDS_DUAL_LINK_ODD_EVEN_PIXELS = BIT(1),
+};
+
+enum lvds_pixel {
+	PIX_R_0		= 0x00,
+	PIX_R_1		= 0x01,
+	PIX_R_2		= 0x02,
+	PIX_R_3		= 0x03,
+	PIX_R_4		= 0x04,
+	PIX_R_5		= 0x05,
+	PIX_R_6		= 0x06,
+	PIX_R_7		= 0x07,
+	PIX_G_0		= 0x08,
+	PIX_G_1		= 0x09,
+	PIX_G_2		= 0x0A,
+	PIX_G_3		= 0x0B,
+	PIX_G_4		= 0x0C,
+	PIX_G_5		= 0x0D,
+	PIX_G_6		= 0x0E,
+	PIX_G_7		= 0x0F,
+	PIX_B_0		= 0x10,
+	PIX_B_1		= 0x11,
+	PIX_B_2		= 0x12,
+	PIX_B_3		= 0x13,
+	PIX_B_4		= 0x14,
+	PIX_B_5		= 0x15,
+	PIX_B_6		= 0x16,
+	PIX_B_7		= 0x17,
+	PIX_H_S		= 0x18,
+	PIX_V_S		= 0x19,
+	PIX_D_E		= 0x1A,
+	PIX_C_E		= 0x1B,
+	PIX_C_I		= 0x1C,
+	PIX_TOG		= 0x1D,
+	PIX_ONE		= 0x1E,
+	PIX_ZER		= 0x1F,
+};
+
+/*
+ * Expected JEIDA-RGB888 data to be sent in LSB format
+ *	    bit6 ............................bit0
+ */
+const enum lvds_pixel lvds_bitmap_jeida_rgb888[5][7] = {
+	{ PIX_ONE, PIX_ONE, PIX_ZER, PIX_ZER, PIX_ZER, PIX_ONE, PIX_ONE },
+	{ PIX_G_2, PIX_R_7, PIX_R_6, PIX_R_5, PIX_R_4, PIX_R_3, PIX_R_2 },
+	{ PIX_B_3, PIX_B_2, PIX_G_7, PIX_G_6, PIX_G_5, PIX_G_4, PIX_G_3 },
+	{ PIX_D_E, PIX_V_S, PIX_H_S, PIX_B_7, PIX_B_6, PIX_B_5, PIX_B_4 },
+	{ PIX_C_E, PIX_B_1, PIX_B_0, PIX_G_1, PIX_G_0, PIX_R_1, PIX_R_0 }
+};
+
+/*
+ * Expected VESA-RGB888 data to be sent in LSB format
+ *	    bit6 ............................bit0
+ */
+const enum lvds_pixel lvds_bitmap_vesa_rgb888[5][7] = {
+	{ PIX_ONE, PIX_ONE, PIX_ZER, PIX_ZER, PIX_ZER, PIX_ONE, PIX_ONE },
+	{ PIX_G_0, PIX_R_5, PIX_R_4, PIX_R_3, PIX_R_2, PIX_R_1, PIX_R_0 },
+	{ PIX_B_1, PIX_B_0, PIX_G_5, PIX_G_4, PIX_G_3, PIX_G_2, PIX_G_1 },
+	{ PIX_D_E, PIX_V_S, PIX_H_S, PIX_B_5, PIX_B_4, PIX_B_3, PIX_B_2 },
+	{ PIX_C_E, PIX_B_7, PIX_B_6, PIX_G_7, PIX_G_6, PIX_R_7, PIX_R_6 }
+};
+
+static inline void lvds_writel(struct stm32_lvds *lvds, u32 reg, u32 val)
+{
+	writel(val, lvds->base + reg);
+}
+
+static inline u32 lvds_readl(struct stm32_lvds *lvds, u32 reg)
+{
+	return readl(lvds->base + reg);
+}
+
+static inline void lvds_set(struct stm32_lvds *lvds, u32 reg, u32 mask)
+{
+	lvds_writel(lvds, reg, lvds_readl(lvds, reg) | mask);
+}
+
+static inline void lvds_clear(struct stm32_lvds *lvds, u32 reg, u32 mask)
+{
+	lvds_writel(lvds, reg, lvds_readl(lvds, reg) & ~mask);
+}
+
+/* Integer mode */
+#define EN_SD		0
+#define EN_TWG		0
+#define DOWN_SPREAD	0
+#define TEST_DIV	70
+
+static u32 pll_get_clkout_khz(u32 clkin_khz, u32 bdiv, u32 mdiv, u32 ndiv)
+{
+	int divisor = ndiv * bdiv;
+
+	/* Prevents from division by 0 */
+	if (!divisor)
+		return 0;
+
+	return clkin_khz * mdiv / divisor;
+}
+
+#define NDIV_MIN	2
+#define NDIV_MAX	6
+#define BDIV_MIN	2
+#define BDIV_MAX	6
+#define MDIV_MIN	1
+#define MDIV_MAX	1023
+
+static int lvds_pll_get_params(u32 clkin_khz, u32 clkout_khz,
+			       u32 *bdiv, u32 *mdiv, u32 *ndiv)
+{
+	u32 i, o, n;
+	u32 delta, best_delta; /* all in khz */
+
+	/* Early checks preventing division by 0 & odd results */
+	if (clkin_khz <= 0 || clkout_khz <= 0)
+		return -EINVAL;
+
+	best_delta = 1000000; /* big started value (1000000khz) */
+
+	for (i = NDIV_MIN; i <= NDIV_MAX; i++) {
+		for (o = BDIV_MIN; o <= BDIV_MAX; o++) {
+			n = DIV_ROUND_CLOSEST(i * o * clkout_khz, clkin_khz);
+			/* Check ndiv according to vco range */
+			if (n < MDIV_MIN || n > MDIV_MAX)
+				continue;
+			/* Check if new delta is better & saves parameters */
+			delta = abs(pll_get_clkout_khz(clkin_khz, i, n, o) - clkout_khz);
+			if (delta < best_delta) {
+				*ndiv = i;
+				*mdiv = n;
+				*bdiv = o;
+				best_delta = delta;
+			}
+			/* fast return in case of "perfect result" */
+			if (!delta)
+				return 0;
+		}
+	}
+
+	return 0;
+}
+
+static int stm32_lvds_pll_enable(struct stm32_lvds *lvds,
+				 struct display_timing *timings,
+				 int phy)
+{
+	u32 pll_in_khz, bdiv = 0, mdiv = 0, ndiv = 0;
+	int ret, val, multiplier;
+
+	/* Release PHY from reset */
+	lvds_set(lvds, LVDS_PxGCR(phy), PHY_GCR_DIV_RSTN | PHY_GCR_RSTZ);
+
+	/* lvds_pll_config */
+	/* Set PLL Slv & Mst configs and timings */
+	pll_in_khz = lvds->refclk / 1000;
+
+	if (lvds->dual_link)
+		multiplier = 2;
+	else
+		multiplier = 1;
+
+	ret = lvds_pll_get_params(pll_in_khz, timings->pixelclock.typ * 7 / 1000 / multiplier,
+				  &bdiv, &mdiv, &ndiv);
+	if (ret)
+		return ret;
+
+	/* Set PLL parameters */
+	lvds_writel(lvds, LVDS_PxPLLCR2(phy), (ndiv << 16) | bdiv);
+	lvds_writel(lvds, LVDS_PxPLLSDCR1(phy), mdiv);
+	lvds_writel(lvds, LVDS_PxPLLTESTCR(phy), TEST_DIV << 16);
+
+	/* Disable TWG and SD: for now, PLL just need to be in integer mode */
+	lvds_clear(lvds, LVDS_PxPLLCR1(phy), EN_TWG | EN_SD);
+
+	/* Power up bias and PLL dividers */
+	lvds_set(lvds, LVDS_PxDCR(phy), POWER_OK);
+
+	lvds_set(lvds, LVDS_PxCMCR1(phy), CM_EN_DL);
+	lvds_set(lvds, LVDS_PxCMCR2(phy), CM_EN_DL4);
+
+	lvds_set(lvds, LVDS_PxPLLCPCR(phy), 0x1);
+	lvds_set(lvds, LVDS_PxBCR3(phy), VM_EN_DL);
+	lvds_set(lvds, LVDS_PxBCR1(phy), EN_BIAS_DL);
+	lvds_set(lvds, LVDS_PxCFGCR(phy), EN_DIG_DL);
+
+	/* lvds_pll_enable */
+	/* PLL lock timing control for the monitor unmask after startup (pll_en) */
+	/* Adjust the value so that the masking window is opened at start-up */
+	/* MST_MON_PLL_LOCK_UNMASK_TUNE */
+	lvds_writel(lvds, LVDS_PxMPLCR(phy), (0x200 - 0x160) << 16);
+
+	lvds_writel(lvds, LVDS_PxBCR2(phy), BIAS_EN);
+
+	lvds_set(lvds, LVDS_PxGCR(phy),
+		 PHY_GCR_DP_CLK_OUT | PHY_GCR_LS_CLK_OUT | PHY_GCR_BIT_CLK_OUT);
+
+	/* TODO hardcoded values for now */
+	lvds_set(lvds, LVDS_PxPLLTESTCR(phy), BIT(8) /* PLL_TEST_DIV_EN */);
+	lvds_set(lvds, LVDS_PxPLLCR1(phy), BIT(8) /* PLL_DIVIDERS_ENABLE */);
+
+	lvds_set(lvds, LVDS_PxSCR(phy), BIT(16) /* SER_DATA_OK */);
+
+	/* Enable the LVDS PLL & wait for its lock */
+	lvds_set(lvds, LVDS_PxPLLCR1(phy), PLL_EN);
+	ret = readl_poll_sleep_timeout(lvds->base + LVDS_PxPLLSR(phy),
+				       val, val & PLL_LOCK, SLEEP_US, TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/* Select MST PHY clock as pixel clock for the LDITX instead of FREF */
+	/* WCLKCR_SLV_CLKPIX_SEL is for dual link */
+	lvds_writel(lvds, LVDS_WCLKCR, WCLKCR_SLV_CLKPIX_SEL);
+
+	lvds_set(lvds, LVDS_PxPLLTESTCR(phy), BIT(0));
+
+	return 0;
+}
+
+static int stm32_lvds_enable(struct udevice *dev,
+			     const struct display_timing *timings)
+{
+	struct stm32_lvds *lvds = dev_get_priv(dev);
+	u32 lvds_cdl1cr = 0;
+	u32 lvds_cdl2cr = 0;
+	u32 lvds_dmlcr = 0;
+	u32 lvds_dmmcr = 0;
+	u32 lvds_cr = 0;
+	int i;
+
+	lvds_clear(lvds, LVDS_CDL1CR, CDLCRx_DISTR0 | CDLCRx_DISTR1 | CDLCRx_DISTR2
+					| CDLCRx_DISTR3 | CDLCRx_DISTR4);
+	lvds_clear(lvds, LVDS_CDL2CR, CDLCRx_DISTR0 | CDLCRx_DISTR1 | CDLCRx_DISTR2
+					| CDLCRx_DISTR3 | CDLCRx_DISTR4);
+
+	/* Set channel distribution */
+	lvds_cr &= ~CR_LKMOD;
+	lvds_cdl1cr = CDL1CR_DEFAULT;
+
+	if (lvds->dual_link) {
+		lvds_cr |= CR_LKMOD;
+		lvds_cdl2cr = CDL2CR_DEFAULT;
+	}
+
+	/* Set signal polarity */
+	if (timings->flags & DISPLAY_FLAGS_DE_LOW)
+		lvds_cr |= CR_DEPOL;
+
+	if (timings->flags & DISPLAY_FLAGS_HSYNC_LOW)
+		lvds_cr |= CR_HSPOL;
+
+	if (timings->flags & DISPLAY_FLAGS_VSYNC_LOW)
+		lvds_cr |= CR_VSPOL;
+
+	/* Set link phase */
+	switch (lvds->dual_link) {
+	case LVDS_DUAL_LINK_EVEN_ODD_PIXELS: /* LKPHA = 0 */
+		lvds_cr &= ~CR_LKPHA;
+		break;
+	case LVDS_DUAL_LINK_ODD_EVEN_PIXELS: /* LKPHA = 1 */
+		lvds_cr |= CR_LKPHA;
+		break;
+	default:
+		dev_dbg(dev, "No phase precised, setting default\n");
+		lvds_cr &= ~CR_LKPHA;
+		break;
+	}
+
+	/* Set Data Mapping */
+	switch (lvds->bus_format) {
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: /* VESA-RGB888 */
+		for (i = 0; i < 5; i++) {
+			lvds_dmlcr = ((lvds_bitmap_vesa_rgb888[i][0])
+				      + (lvds_bitmap_vesa_rgb888[i][1] << 5)
+				      + (lvds_bitmap_vesa_rgb888[i][2] << 10)
+				      + (lvds_bitmap_vesa_rgb888[i][3] << 15));
+			lvds_dmmcr = ((lvds_bitmap_vesa_rgb888[i][4])
+				      + (lvds_bitmap_vesa_rgb888[i][5] << 5)
+				      + (lvds_bitmap_vesa_rgb888[i][6] << 10));
+
+			/* Write registers at the end of computations */
+			lvds_writel(lvds, LVDS_DMLCR(i), lvds_dmlcr);
+			lvds_writel(lvds, LVDS_DMMCR(i), lvds_dmmcr);
+		}
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: /* JEIDA-RGB888 */
+		for (i = 0; i < 5; i++) {
+			lvds_dmlcr = ((lvds_bitmap_jeida_rgb888[i][0])
+				      + (lvds_bitmap_jeida_rgb888[i][1] << 5)
+				      + (lvds_bitmap_jeida_rgb888[i][2] << 10)
+				      + (lvds_bitmap_jeida_rgb888[i][3] << 15));
+			lvds_dmmcr = ((lvds_bitmap_jeida_rgb888[i][4])
+				      + (lvds_bitmap_jeida_rgb888[i][5] << 5)
+				      + (lvds_bitmap_jeida_rgb888[i][6] << 10));
+
+			/* Write registers at the end of computations */
+			lvds_writel(lvds, LVDS_DMLCR(i), lvds_dmlcr);
+			lvds_writel(lvds, LVDS_DMMCR(i), lvds_dmmcr);
+		}
+		break;
+	default:
+		dev_dbg(dev, "Unsupported LVDS bus format 0x%04x\n", lvds->bus_format);
+	}
+
+	/* Turn the output on */
+	lvds_cr |= CR_LVDSEN;
+
+	/* Commit config to registers */
+	lvds_set(lvds, LVDS_CR, lvds_cr);
+	lvds_writel(lvds, LVDS_CDL1CR, lvds_cdl1cr);
+	lvds_writel(lvds, LVDS_CDL2CR, lvds_cdl2cr);
+
+	return 0;
+}
+
+static int stm32_lvds_attach(struct udevice *dev)
+{
+	struct stm32_lvds *lvds = dev_get_priv(dev);
+	struct display_timing timings;
+	int ret;
+
+	ret = panel_get_display_timing(lvds->panel, &timings);
+	if (ret) {
+		ret = ofnode_decode_display_timing(dev_ofnode(lvds->panel),
+						   0, &timings);
+		if (ret) {
+			dev_err(dev, "decode display timing error %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = stm32_lvds_enable(dev, &timings);
+
+	return ret;
+}
+
+static int stm32_lvds_set_backlight(struct udevice *dev, int percent)
+{
+	struct stm32_lvds *lvds = dev_get_priv(dev);
+	int ret;
+
+	ret = panel_enable_backlight(lvds->panel);
+	if (ret) {
+		dev_err(dev, "panel %s enable backlight error %d\n",
+			lvds->panel->name, ret);
+	}
+
+	return ret;
+}
+
+static int lvds_handle_pixel_order(struct stm32_lvds *lvds)
+{
+	ofnode parent, panel_port0, panel_port1;
+	bool even_pixels, odd_pixels;
+	int port0, port1;
+
+	/*
+	 * In case we are operating in single link,
+	 * there is only one port linked to the LVDS.
+	 * Check whether we are in this case and exit if yes.
+	 */
+	parent = ofnode_find_subnode(dev_ofnode(lvds->panel), "ports");
+	if (!ofnode_valid(parent))
+		return 0;
+
+	panel_port0 = ofnode_first_subnode(parent);
+	if (!ofnode_valid(panel_port0))
+		return -EPIPE;
+
+	even_pixels = ofnode_read_bool(panel_port0, "dual-lvds-even-pixels");
+	odd_pixels = ofnode_read_bool(panel_port0, "dual-lvds-odd-pixels");
+	if (even_pixels && odd_pixels)
+		return -EINVAL;
+
+	port0 = even_pixels ? LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
+		LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
+
+	panel_port1 = ofnode_next_subnode(panel_port0);
+	if (!ofnode_valid(panel_port1))
+		return -EPIPE;
+
+	even_pixels = ofnode_read_bool(panel_port1, "dual-lvds-even-pixels");
+	odd_pixels = ofnode_read_bool(panel_port1, "dual-lvds-odd-pixels");
+	if (even_pixels && odd_pixels)
+		return -EINVAL;
+
+	port1 = even_pixels ? LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
+		LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
+
+	/*
+	 * A valid dual-LVDS bus is found when one port is marked with
+	 * "dual-lvds-even-pixels", and the other port is marked with
+	 * "dual-lvds-odd-pixels", bail out if the markers are not right.
+	 */
+	if (port0 + port1 != LVDS_DUAL_LINK_EVEN_ODD_PIXELS + LVDS_DUAL_LINK_ODD_EVEN_PIXELS)
+		return -EINVAL;
+
+	return port0;
+}
+
+static int stm32_lvds_probe(struct udevice *dev)
+{
+	struct stm32_lvds *priv = dev_get_priv(dev);
+	struct display_timing timings;
+	struct reset_ctl rst;
+	struct clk pclk, refclk;
+	const char *data_mapping;
+	int ret;
+
+	priv->base = dev_read_addr_ptr(dev);
+	if ((fdt_addr_t)priv->base == FDT_ADDR_T_NONE) {
+		dev_err(dev, "Unable to read LVDS base address\n");
+		return -EINVAL;
+	}
+
+	ret = clk_get_by_name(dev, "pclk", &pclk);
+	if (ret) {
+		dev_err(dev, "Unable to get peripheral clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_enable(&pclk);
+	if (ret) {
+		dev_err(dev, "Failed to enable peripheral clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_get_by_name(dev, "ref", &refclk);
+	if (ret) {
+		dev_err(dev, "Unable to get reference clock: %d\n", ret);
+		goto err_clk;
+	}
+
+	ret = clk_enable(&refclk);
+	if (ret) {
+		dev_err(dev, "Failed to enable reference clock: %d\n", ret);
+		goto err_clk;
+	}
+
+	priv->refclk = (unsigned int)clk_get_rate(&refclk);
+
+	ret = reset_get_by_index(dev, 0, &rst);
+	if (ret) {
+		dev_err(dev, "Failed to get LVDS reset: %d\n", ret);
+		goto err_rst;
+	}
+
+	reset_deassert(&rst);
+
+	ret = uclass_get_device_by_driver(UCLASS_PANEL,
+					  DM_DRIVER_GET(simple_panel), &priv->panel);
+	if (ret) {
+		dev_err(dev, "panel device error %d\n", ret);
+		goto err_rst;
+	}
+
+	ret = panel_get_display_timing(priv->panel, &timings);
+	if (ret) {
+		ret = ofnode_decode_display_timing(dev_ofnode(priv->panel),
+						   0, &timings);
+		if (ret) {
+			dev_err(dev, "decode display timing error %d\n", ret);
+			goto err_rst;
+		}
+	}
+
+	data_mapping = ofnode_read_string(dev_ofnode(priv->panel), "data-mapping");
+	if (!strcmp(data_mapping, "vesa-24"))
+		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
+	else if (!strcmp(data_mapping, "jeida-24"))
+		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
+	else
+		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
+
+	/* Handle dual link config */
+	priv->dual_link = lvds_handle_pixel_order(priv);
+	if (priv->dual_link < 0)
+		goto err_rst;
+
+	if (priv->dual_link > 0) {
+		ret = stm32_lvds_pll_enable(priv, &timings, LVDS_PHY_SLAVE);
+		if (ret)
+			goto err_rst;
+	}
+
+	ret = stm32_lvds_pll_enable(priv, &timings, LVDS_PHY_MASTER);
+	if (ret)
+		goto err_rst;
+
+	return 0;
+
+err_rst:
+	clk_disable(&refclk);
+err_clk:
+	clk_disable(&pclk);
+
+	return ret;
+}
+
+static const struct video_bridge_ops stm32_lvds_ops = {
+	.attach = stm32_lvds_attach,
+	.set_backlight = stm32_lvds_set_backlight,
+};
+
+static const struct udevice_id stm32_lvds_ids[] = {
+	{.compatible = "st,stm32mp25-lvds"},
+	{}
+};
+
+U_BOOT_DRIVER(stm32_lvds) = {
+	.name		= "stm32-display-lvds",
+	.id		= UCLASS_VIDEO_BRIDGE,
+	.of_match	= stm32_lvds_ids,
+	.ops		= &stm32_lvds_ops,
+	.probe		= stm32_lvds_probe,
+	.priv_auto	= sizeof(struct stm32_lvds),
+};

-- 
2.25.1

[PATCH 4/8] video: stm32: ltdc: support new hardware version for STM32MP25 SoC

[Raphael Gallais-Pou]

STM32MP2 SoCs feature a new version of the LTDC IP.  This new version
features a bus clock, as well as a 150MHz pad frequency.  Add its
compatible to the list of device to probe and handle quirks.  The new
hardware version features a bus clock.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/video/stm32/stm32_ltdc.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/video/stm32/stm32_ltdc.c b/drivers/video/stm32/stm32_ltdc.c
index 0a062c8939dbe49b11aa50f5ca9701bdbe5c5b0b..d9ba5c4ef53558fe8b5565d47b022699d9e4ea0e 100644
--- a/drivers/video/stm32/stm32_ltdc.c
+++ b/drivers/video/stm32/stm32_ltdc.c
@@ -262,6 +262,7 @@ static const u32 layer_regs_a2[] = {
 #define HWVER_10300 0x010300
 #define HWVER_20101 0x020101
 #define HWVER_40100 0x040100
+#define HWVER_40101 0x040101
 
 enum stm32_ltdc_pix_fmt {
 	PF_ARGB8888 = 0,	/* ARGB [32 bits] */
@@ -529,7 +530,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
 	struct udevice *bridge = NULL;
 	struct udevice *panel = NULL;
 	struct display_timing timings;
-	struct clk pclk;
+	struct clk pclk, bclk;
 	struct reset_ctl rst;
 	ulong rate;
 	int ret;
@@ -540,7 +541,21 @@ static int stm32_ltdc_probe(struct udevice *dev)
 		return -EINVAL;
 	}
 
-	ret = clk_get_by_index(dev, 0, &pclk);
+	ret = clk_get_by_name(dev, "bus", &bclk);
+	if (ret) {
+		if (ret != -ENODATA) {
+			dev_err(dev, "bus clock get error %d\n", ret);
+			return ret;
+		}
+	} else {
+		ret = clk_enable(&bclk);
+		if (ret) {
+			dev_err(dev, "bus clock enable error %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = clk_get_by_name(dev, "lcd", &pclk);
 	if (ret) {
 		dev_err(dev, "peripheral clock get error %d\n", ret);
 		return ret;
@@ -566,6 +581,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
 		priv->pix_fmt_hw = pix_fmt_a1;
 		break;
 	case HWVER_40100:
+	case HWVER_40101:
 		priv->layer_regs = layer_regs_a2;
 		priv->pix_fmt_hw = pix_fmt_a2;
 		break;
@@ -688,6 +704,7 @@ static int stm32_ltdc_bind(struct udevice *dev)
 
 static const struct udevice_id stm32_ltdc_ids[] = {
 	{ .compatible = "st,stm32-ltdc" },
+	{ .compatible = "st,stm32mp251-ltdc" },
 	{ }
 };
 

-- 
2.25.1

[PATCH 5/8] video: stm32: ltdc: properly search the first available panel

[Raphael Gallais-Pou]

Initially there was only one DSI bridge with one panel attached to this
device. This explained the call to uclass_first_device_err(UCLASS_PANEL,
...) which worked fine at the time.

Now that multiple bridges and panels, with different technologies, can
be plugged onto the board this way to get the panel device is outdated.

The lookup is done is two steps. First we cirle through the
UCLASS_VIDEO_BRIDGE, and once we get one, we search through its
endpoints until we get a UCLASS_PANEL device available.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/video/stm32/stm32_ltdc.c | 136 +++++++++++++++++++++++++++++++++++----
 1 file changed, 125 insertions(+), 11 deletions(-)

diff --git a/drivers/video/stm32/stm32_ltdc.c b/drivers/video/stm32/stm32_ltdc.c
index d9ba5c4ef53558fe8b5565d47b022699d9e4ea0e..cdcfde678fb59ad884125e7c26f43710d76ba246 100644
--- a/drivers/video/stm32/stm32_ltdc.c
+++ b/drivers/video/stm32/stm32_ltdc.c
@@ -17,6 +17,7 @@
 #include <video_bridge.h>
 #include <asm/io.h>
 #include <dm/device-internal.h>
+#include <dm/uclass-internal.h>
 #include <dm/device_compat.h>
 #include <linux/bitops.h>
 #include <linux/printk.h>
@@ -495,6 +496,101 @@ static void stm32_ltdc_set_layer1(struct stm32_ltdc_priv *priv, ulong fb_addr)
 	setbits_le32(priv->regs + LTDC_L1CR, LXCR_LEN);
 }
 
+static int stm32_ltdc_get_remote_device(struct udevice *dev, ofnode ep_node,
+					enum uclass_id id, struct udevice **remote_dev)
+{
+	u32 remote_phandle;
+	ofnode remote;
+	int ret = 0;
+
+	ret = ofnode_read_u32(ep_node, "remote-endpoint", &remote_phandle);
+	if (ret) {
+		dev_err(dev, "%s(%s): Could not find remote-endpoint property\n",
+			__func__, dev_read_name(dev));
+		return ret;
+	}
+
+	remote = ofnode_get_by_phandle(remote_phandle);
+	if (!ofnode_valid(remote))
+		return -EINVAL;
+
+	while (ofnode_valid(remote)) {
+		remote = ofnode_get_parent(remote);
+		if (!ofnode_valid(remote)) {
+			dev_dbg(dev, "%s(%s): no uclass_id %d for remote-endpoint\n",
+				__func__, dev_read_name(dev), id);
+			continue;
+		}
+
+		ret = uclass_find_device_by_ofnode(id, remote, remote_dev);
+		if (*remote_dev && !ret) {
+			ret = uclass_get_device_by_ofnode(id, remote, remote_dev);
+			if (ret)
+				dev_dbg(dev, "%s(%s): failed to get remote device %s\n",
+					__func__, dev_read_name(dev), dev_read_name(*remote_dev));
+			break;
+		}
+	};
+
+	return ret;
+}
+
+static int stm32_ltdc_get_panel(struct udevice *dev, struct udevice **panel)
+{
+	ofnode ep_node, node, ports;
+	int ret = 0;
+
+	if (!dev)
+		return -EINVAL;
+
+	ports = ofnode_find_subnode(dev_ofnode(dev), "ports");
+	if (!ofnode_valid(ports)) {
+		dev_err(dev, "Remote bridge subnode\n");
+		return ret;
+	}
+
+	for (node = ofnode_first_subnode(ports);
+	     ofnode_valid(node);
+	     node = dev_read_next_subnode(node)) {
+		ep_node = ofnode_first_subnode(node);
+		if (!ofnode_valid(ep_node))
+			continue;
+
+		ret = stm32_ltdc_get_remote_device(dev, ep_node, UCLASS_PANEL, panel);
+	}
+
+	/* Sanity check, we can get out of the loop without having a clean ofnode */
+	if (!(*panel))
+		ret = -EINVAL;
+	else
+		if (!ofnode_valid(dev_ofnode(*panel)))
+			ret = -EINVAL;
+
+	return ret;
+}
+
+static int stm32_ltdc_display_init(struct udevice *dev, ofnode *ep_node,
+				   struct udevice **panel, struct udevice **bridge)
+{
+	int ret;
+
+	if (*panel)
+		return -EINVAL;
+
+	if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
+		ret = stm32_ltdc_get_remote_device(dev, *ep_node, UCLASS_VIDEO_BRIDGE, bridge);
+		if (ret)
+			return ret;
+
+		ret = stm32_ltdc_get_panel(*bridge, panel);
+	} else {
+		/* no bridge, search a panel from display controller node */
+		ret = stm32_ltdc_get_remote_device(dev, *ep_node, UCLASS_PANEL, panel);
+	}
+
+	return ret;
+}
+
 #if IS_ENABLED(CONFIG_TARGET_STM32F469_DISCOVERY)
 static int stm32_ltdc_alloc_fb(struct udevice *dev)
 {
@@ -532,6 +628,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
 	struct display_timing timings;
 	struct clk pclk, bclk;
 	struct reset_ctl rst;
+	ofnode node, port;
 	ulong rate;
 	int ret;
 
@@ -568,7 +665,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
 	}
 
 	priv->hw_version = readl(priv->regs + LTDC_IDR);
-	debug("%s: LTDC hardware 0x%x\n", __func__, priv->hw_version);
+	dev_dbg(dev, "%s: LTDC hardware 0x%x\n", __func__, priv->hw_version);
 
 	switch (priv->hw_version) {
 	case HWVER_10200:
@@ -589,13 +686,35 @@ static int stm32_ltdc_probe(struct udevice *dev)
 		return -ENODEV;
 	}
 
-	ret = uclass_first_device_err(UCLASS_PANEL, &panel);
-	if (ret) {
-		if (ret != -ENODEV)
-			dev_err(dev, "panel device error %d\n", ret);
-		return ret;
+	/*
+	 * Try all the ports until one working.
+	 *
+	 * This is done in two times. First is checks for the
+	 * UCLASS_VIDEO_BRIDGE available, and then for this bridge
+	 * it scans for a UCLASS_PANEL.
+	 */
+
+	port = dev_read_subnode(dev, "port");
+	if (!ofnode_valid(port)) {
+		dev_err(dev, "%s(%s): 'port' subnode not found\n",
+			__func__, dev_read_name(dev));
+		return -EINVAL;
 	}
 
+	for (node = ofnode_first_subnode(port);
+	     ofnode_valid(node);
+	     node = dev_read_next_subnode(node)) {
+		ret = stm32_ltdc_display_init(dev, &node, &panel, &bridge);
+		if (ret)
+			dev_dbg(dev, "Device failed ret=%d\n", ret);
+		else
+			break;
+	}
+
+	/* Sanity check */
+	if (ret)
+		return ret;
+
 	ret = panel_get_display_timing(panel, &timings);
 	if (ret) {
 		ret = ofnode_decode_display_timing(dev_ofnode(panel),
@@ -624,11 +743,6 @@ static int stm32_ltdc_probe(struct udevice *dev)
 	reset_deassert(&rst);
 
 	if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
-		ret = uclass_get_device(UCLASS_VIDEO_BRIDGE, 0, &bridge);
-		if (ret)
-			dev_dbg(dev,
-				"No video bridge, or no backlight on bridge\n");
-
 		if (bridge) {
 			ret = video_bridge_attach(bridge);
 			if (ret) {

-- 
2.25.1

[PATCH 6/8] ARM: dts: stm32: use LTDC and LVDS nodes before relocation in stm32mp25-u-boot

[Raphael Gallais-Pou]

Use LTDC and LVDS nodes in all boot phases.  This is specially useful
before U-Boot relocation.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 arch/arm/dts/stm32mp25-u-boot.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/dts/stm32mp25-u-boot.dtsi b/arch/arm/dts/stm32mp25-u-boot.dtsi
index d9aeeb6d510df9dea9700148cf8a8ad5efcfd4f9..3ac35c4a6bc27c72eeeb532742fd4412b7aa1b85 100644
--- a/arch/arm/dts/stm32mp25-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp25-u-boot.dtsi
@@ -93,6 +93,14 @@
 	bootph-all;
 };
 
+&ltdc {
+	bootph-all;
+};
+
+&lvds {
+	bootph-all;
+};
+
 &pinctrl {
 	bootph-all;
 };

-- 
2.25.1

[PATCH 7/8] ARM: dts: stm32: remove panel property in stm32mp257f-ev1-u-boot

[Raphael Gallais-Pou]

SCMI regulators are not yet implemented in OP-TEE.  This leads to an
invalid argument error when retrieving the 'power-supply' property.
Ultimately this chains into the panel not probing and makes the whole
display pipe fail.

Remove the property in the upstream device-tree to allow display pipe to
boot.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
index 9a566e18d3fd9b0cf4e21605f2cb060b49fadcb6..7a4b623664a1e167ccc0f4c73a979a083337d7a2 100644
--- a/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
@@ -12,6 +12,10 @@
 	};
 };
 
+&panel_lvds {
+	/delete-property/ power-supply;
+};
+
 &usart2 {
 	bootph-all;
 };

-- 
2.25.1

[PATCH 8/8] configs: stm32mp25: enable LVDS display support

[Raphael Gallais-Pou]

Compile VIDEO_STM32 and VIDEO_STM32_LVDS by default.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 configs/stm32mp25_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/stm32mp25_defconfig b/configs/stm32mp25_defconfig
index 14619ffd96c71028b508897c77f9c8a747166683..bb9bb935e2d427fc515b83fb32ed93cdcac34c1d 100644
--- a/configs/stm32mp25_defconfig
+++ b/configs/stm32mp25_defconfig
@@ -76,6 +76,9 @@ CONFIG_SERIAL_RX_BUFFER=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
 # CONFIG_OPTEE_TA_AVB is not set
+CONFIG_VIDEO=y
+CONFIG_VIDEO_STM32=y
+CONFIG_VIDEO_STM32_LVDS=y
 CONFIG_WDT=y
 CONFIG_WDT_STM32MP=y
 CONFIG_WDT_ARM_SMC=y

-- 
2.25.1

Re: [PATCH 1/8] ofnode: support panel-timings in ofnode_decode_display_timing

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> The "Display Timings" in panel-common.yaml can be provided by 2 properties
> - panel-timing: when display panels are restricted to a single resolution
>                 the "panel-timing" node expresses the required timings.
> - display-timings: several resolutions with different timings are supported
>                    with several timing subnode of "display-timings" node
> 
> This patch update the parsing function to handle this 2 possibility
> when index = 0.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  drivers/core/ofnode.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index e040e3f2806ffe74c58dcd82f36307351acd5a99..5a721b46e5a3214e7bd437739776362c2d22a3c9 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -1221,13 +1221,16 @@ int ofnode_decode_display_timing(ofnode parent, int index,
>  	int ret = 0;
>  
>  	timings = ofnode_find_subnode(parent, "display-timings");
> -	if (!ofnode_valid(timings))
> -		return -EINVAL;
> -
> -	i = 0;
> -	ofnode_for_each_subnode(node, timings) {
> -		if (i++ == index)
> -			break;
> +	if (ofnode_valid(timings)) {
> +		i = 0;
> +		ofnode_for_each_subnode(node, timings) {
> +			if (i++ == index)
> +				break;
> +		}
> +	} else {
> +		if (index != 0)
> +			return -EINVAL;
> +		node = ofnode_find_subnode(parent, "panel-timing");
>  	}
>  
>  	if (!ofnode_valid(node))
> 

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

Re: [PATCH 2/8] video: simple_panel: add support for "panel-lvds" display

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> Add the compatible "panel-lvds" for simple-panel driver in U-Boot.  In
> Linux this compatible is managed by the driver
> drivers/gpu/drm/panel/panel-lvds.c but in U-Boot the specific LVDS
> features (bus_format/bus_flags) are not supported.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  drivers/video/simple_panel.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/simple_panel.c b/drivers/video/simple_panel.c
> index b6c5b058b2e967bbbd47ab3c3ce5ca52c7804409..0f23df701bc3c40ea49380bbfa3743ee592d8bd3 100644
> --- a/drivers/video/simple_panel.c
> +++ b/drivers/video/simple_panel.c
> @@ -191,6 +191,7 @@ static const struct mipi_dsi_panel_plat panasonic_vvx10f004b00 = {
>  
>  static const struct udevice_id simple_panel_ids[] = {
>  	{ .compatible = "simple-panel" },
> +	{ .compatible = "panel-lvds" },
>  	{ .compatible = "auo,b133xtn01" },
>  	{ .compatible = "auo,b116xw03" },
>  	{ .compatible = "auo,b133htn01" },
> 

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

Re: [PATCH 3/8] video: stm32: STM32 driver support for LVDS

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> The LVDS Display Interface Transmitter handles the LVDS protocol:
> it maps the pixels received from the upstream Pixel-DMA (LTDC)
> onto the LVDS PHY.
> 
> The LVDS controller driver supports the following high-level features:
>         • FDP-Link-I and OpenLDI (v0.95) protocols
>         • Single-Link or Dual-Link operation
>         • Single-Display or Double-Display (with the same content
>           duplicated on both)
>         • Flexible Bit-Mapping, including JEIDA and VESA
>         • RGB888 or RGB666 output
>         • Synchronous design, with one input pixel per clock cycle
>         • No resolution limitation.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  MAINTAINERS                      |   1 +
>  doc/board/st/st-dt.rst           |   1 +
>  drivers/video/stm32/Kconfig      |   9 +
>  drivers/video/stm32/Makefile     |   1 +
>  drivers/video/stm32/stm32_lvds.c | 673 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 685 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3fb163aa1db1dba249e60b40ab3e785db4fde5d3..604ba4ca04c1079b14cd8f4c1fff4c12da0e278d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -728,6 +728,7 @@ F:	drivers/serial/serial_stm32.*
>  F:	drivers/spi/stm32_qspi.c
>  F:	drivers/spi/stm32_spi.c
>  F:	drivers/video/stm32/stm32_ltdc.c
> +F:	drivers/video/stm32/stm32_lvds.c
>  F:	drivers/watchdog/stm32mp_wdt.c
>  F:	include/dt-bindings/clock/stm32fx-clock.h
>  F:	include/dt-bindings/clock/stm32mp*
> diff --git a/doc/board/st/st-dt.rst b/doc/board/st/st-dt.rst
> index 2a285c81807ee26924948d8c2497f11dfe45b6e6..28cada97e72da711408a1aaaa928ad3faa51162e 100644
> --- a/doc/board/st/st-dt.rst
> +++ b/doc/board/st/st-dt.rst
> @@ -25,6 +25,7 @@ kernel binding directory = Documentation/devicetree/bindings/
>  * display
>  	- display/st,stm32-dsi.yaml
>  	- display/st,stm32-ltdc.yaml
> +        - display/st,stm32mp25-lvds.yaml
>  * gpio
>  	- pinctrl/st,stm32-pinctrl.yaml
>  * hwlock
> diff --git a/drivers/video/stm32/Kconfig b/drivers/video/stm32/Kconfig
> index c354c402c288b4c004bd2b5cc74a4d00beef2773..4cb8a841caf51aaba5115b9fcddb967f73a3d9b4 100644
> --- a/drivers/video/stm32/Kconfig
> +++ b/drivers/video/stm32/Kconfig
> @@ -23,6 +23,15 @@ config VIDEO_STM32_DSI
>  	  This option enables support DSI internal bridge which can be used on
>  	  devices which have DSI devices connected.
>  
> +config VIDEO_STM32_LVDS
> +	bool "Enable STM32 LVDS video support"
> +	depends on VIDEO_STM32
> +	select VIDEO_BRIDGE
> +	select VIDEO_DW_MIPI_DSI
> +	help
> +	  This enables Low Voltage Differential Signaling (LVDS) display
> +	  support.
> +
>  config VIDEO_STM32_MAX_XRES
>  	int "Maximum horizontal resolution (for memory allocation purposes)"
>  	depends on VIDEO_STM32
> diff --git a/drivers/video/stm32/Makefile b/drivers/video/stm32/Makefile
> index f8b42d1a4d126a31aa11ce820e829b133ca3bb2d..059d9000c1d7762d06bff36d5408143a34848227 100644
> --- a/drivers/video/stm32/Makefile
> +++ b/drivers/video/stm32/Makefile
> @@ -7,3 +7,4 @@
>  
>  obj-${CONFIG_VIDEO_STM32} = stm32_ltdc.o
>  obj-${CONFIG_VIDEO_STM32_DSI} += stm32_dsi.o
> +obj-${CONFIG_VIDEO_STM32_LVDS} += stm32_lvds.o
> diff --git a/drivers/video/stm32/stm32_lvds.c b/drivers/video/stm32/stm32_lvds.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1e3e740122dfee0c947819532da64f0d99327bde
> --- /dev/null
> +++ b/drivers/video/stm32/stm32_lvds.c
> @@ -0,0 +1,673 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
> + * Author(s): Raphaël Gallais-Pou <raphael.gallais-pou@foss.st.com> for STMicroelectronics.
> + *
> + * This Low Voltage Differential Signal controller driver is based on the Linux Kernel driver from
> + * drivers/gpu/drm/stm/ltdc.c
> + */
> +
> +#define LOG_CATEGORY UCLASS_VIDEO_BRIDGE
> +
> +#include <clk.h>
> +#include <dm.h>
> +#include <log.h>
> +#include <media_bus_format.h>
> +#include <panel.h>
> +#include <reset.h>
> +#include <video.h>
> +#include <video_bridge.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <dm/ofnode.h>
> +#include <linux/iopoll.h>
> +
> +/* LVDS Host registers */
> +#define LVDS_CR		0x0000  /* configuration register */
> +#define LVDS_DMLCR0	0x0004  /* data mapping lsb configuration register 0	*/
> +#define LVDS_DMMCR0	0x0008  /* data mapping msb configuration register 0	*/
> +#define LVDS_DMLCR1	0x000C  /* data mapping lsb configuration register 1	*/
> +#define LVDS_DMMCR1	0x0010  /* data mapping msb configuration register 1	*/
> +#define LVDS_DMLCR2	0x0014  /* data mapping lsb configuration register 2	*/
> +#define LVDS_DMMCR2	0x0018  /* data mapping msb configuration register 2	*/
> +#define LVDS_DMLCR3	0x001C  /* data mapping lsb configuration register 3	*/
> +#define LVDS_DMMCR3	0x0020  /* data mapping msb configuration register 3	*/
> +#define LVDS_DMLCR4	0x0024  /* data mapping lsb configuration register 4	*/
> +#define LVDS_DMMCR4	0x0028  /* data mapping msb configuration register 4	*/
> +#define LVDS_DMLCR(id)	(LVDS_DMLCR0 + 8U * (id))
> +#define LVDS_DMMCR(id)	(LVDS_DMMCR0 + 8U * (id))
> +#define LVDS_CDL1CR	0x002C  /* channel distrib link 1 configuration register	*/
> +#define LVDS_CDL2CR	0x0030  /* channel distrib link 2 configuration register	*/
> +
> +#define CDL1CR_DEFAULT	0x4321
> +#define CDL2CR_DEFAULT	0x59876
> +
> +/* LVDS Host registers */
> +#define LVDS_PHY_MASTER	0x0
> +#define LVDS_PHY_SLAVE	0x100
> +
> +/* phy parameter can only be one of those two above */
> +#define LVDS_PxGCR(phy)		((phy) + 0x1000)   /* Global Control Register	*/

All define must be in uppercase, you are mixing lower/upper case which is not encouraged.
Same remark below .

> +#define LVDS_PxCMCR1(phy)	((phy) + 0x100C)   /* Current Mode Control Register 1 */
> +#define LVDS_PxCMCR2(phy)	((phy) + 0x1010)  /* Current Mode Control Register 2 */
> +#define LVDS_PxSCR(phy)		((phy) + 0x1020)  /* Serial Control Register	*/
> +#define LVDS_PxBCR1(phy)	((phy) + 0x102C)  /* Bias Control Register 1	*/
> +#define LVDS_PxBCR2(phy)	((phy) + 0x1030)  /* Bias Control Register 2	*/
> +#define LVDS_PxBCR3(phy)	((phy) + 0x1034)  /* Bias Control Register 3	*/
> +#define LVDS_PxMPLCR(phy)	((phy) + 0x1064)  /* Monitor PLL Lock Control Register */
> +#define LVDS_PxDCR(phy)		((phy) + 0x1084)  /* Debug Control Register	*/
> +#define LVDS_PxSSR1(phy)	((phy) + 0x1088)  /* Spare Status Register 1	*/
> +#define LVDS_PxCFGCR(phy)	((phy) + 0x10A0)  /* Configuration Control Register */
> +#define LVDS_PxPLLCR1(phy)	((phy) + 0x10C0)  /* PLL_MODE 1 Control Register	*/
> +#define LVDS_PxPLLCR2(phy)	((phy) + 0x10C4)  /* PLL_MODE 2 Control Register	*/
> +#define LVDS_PxPLLSR(phy)	((phy) + 0x10C8)  /* PLL Status Register	*/
> +#define LVDS_PxPLLSDCR1(phy)	((phy) + 0x10CC)  /* PLL_SD_1 Control Register	*/
> +#define LVDS_PxPLLSDCR2(phy)	((phy) + 0x10D0)  /* PLL_SD_2 Control Register	*/
> +#define LVDS_PxPLLTWGCR1(phy)	((phy) + 0x10D4)  /* PLL_TWG_1 Control Register	*/
> +#define LVDS_PxPLLTWGCR2(phy)	((phy) + 0x10D8)  /* PLL_TWG_2 Control Register	*/
> +#define LVDS_PxPLLCPCR(phy)	((phy) + 0x10E0)  /* PLL_CP Control Register	*/
> +#define LVDS_PxPLLTESTCR(phy)	((phy) + 0x10E8)  /* PLL_TEST Control Register	*/
> +
> +/* LVDS Wrapper registers */
> +#define LVDS_WCLKCR	0x11B0  /* Wrapper clock control register */
> +#define LVDS_HWCFGR	0x1FF0  /* HW configuration register	*/
> +#define LVDS_VERR	0x1FF4  /* Version register	*/
> +#define LVDS_IPIDR	0x1FF8  /* Identification register	*/
> +#define LVDS_SIDR	0x1FFC  /* Size Identification register	*/
> +
> +#define CR_LVDSEN	BIT(0)  /* LVDS PHY Enable */
> +#define CR_HSPOL	BIT(1)  /* HS Polarity (horizontal sync) */
> +#define CR_VSPOL	BIT(2)  /* VS Polarity (vertical sync) */
> +#define CR_DEPOL	BIT(3)  /* DE Polarity (data enable) */
> +#define CR_CI		BIT(4)  /* Control Internal (software controlled bit) */
> +#define CR_LKMOD	BIT(5)  /* Link Mode, for both Links */
> +#define CR_LKPHA	BIT(6)  /* Link Phase, for both Links */
> +#define CR_LK1POL	GENMASK(20, 16)  /* Link-1 output Polarity */
> +#define CR_LK2POL	GENMASK(25, 21)  /* Link-2 output Polarity */
> +
> +#define DMMCRx_MAP0	GENMASK(4, 0)
> +#define DMMCRx_MAP1	GENMASK(9, 5)
> +#define DMMCRx_MAP2	GENMASK(14, 10)
> +#define DMMCRx_MAP3	GENMASK(19, 15)
> +#define DMLCRx_MAP4	GENMASK(4, 0)
> +#define DMLCRx_MAP5	GENMASK(9, 5)
> +#define DMLCRx_MAP6	GENMASK(14, 10)
> +
> +#define CDLCRx_DISTR0	GENMASK(3, 0)
> +#define CDLCRx_DISTR1	GENMASK(7, 4)
> +#define CDLCRx_DISTR2	GENMASK(11, 8)
> +#define CDLCRx_DISTR3	GENMASK(15, 12)
> +#define CDLCRx_DISTR4	GENMASK(19, 16)
> +
> +#define FREF_INDEX	0
> +#define NDIV_INDEX	1
> +#define FPFD_INDEX	2
> +#define MDIV_INDEX	3
> +#define FVCO_INDEX	4
> +#define BDIV_INDEX	5
> +#define FBIT_INDEX	6
> +#define FLS_INDEX	7
> +#define FDP_INDEX	8
> +
> +#define PHY_GCR_BIT_CLK_OUT	BIT(0)
> +#define PHY_GCR_LS_CLK_OUT	BIT(4)
> +#define PHY_GCR_DP_CLK_OUT	BIT(8)
> +#define PHY_GCR_RSTZ		BIT(24)
> +#define PHY_GCR_DIV_RSTN	BIT(25)
> +
> +#define PHY_PxPLLTESTCR_TDIV	GENMASK(25, 16)
> +#define PHY_PxPLLCR2_NDIV	GENMASK(25, 16)
> +#define PHY_PxPLLCR2_BDIV	GENMASK(9, 0)
> +#define PHY_PxPLLSDCR1_MDIV	GENMASK(9, 0)
> +
> +#define PLL_EN		BIT(0)
> +#define PLL_LOCK	BIT(0)
> +#define CM_EN_DL	(BIT(28) | BIT(20) | BIT(12) | BIT(4))
> +#define CM_EN_DL4	BIT(4)
> +#define VM_EN_DL	(BIT(16) | BIT(12) | BIT(8) | BIT(4) | BIT(0))
> +#define EN_BIAS_DL	(BIT(16) | BIT(12) | BIT(8) | BIT(4) | BIT(0))
> +#define EN_DIG_DL	GENMASK(4, 0)
> +#define BIAS_EN		BIT(28)
> +#define POWER_OK	BIT(12)
> +
> +#define WCLKCR_SLV_CLKPIX_SEL	BIT(0)
> +#define WCLKCR_SRCSEL		BIT(8)
> +
> +/* Sleep & timeout for pll lock/unlock */
> +#define SLEEP_US	1000
> +#define TIMEOUT_US	20000000
> +
> +#define PHY_SLV_OFS	0x100
> +
> +struct stm32_lvds {
> +	void __iomem *base;
> +	struct udevice *panel;
> +	u32 refclk;
> +	int dual_link;
> +	int bus_format;
> +};
> +
> +/*
> + * enum lvds_pixels_order - Pixel order of an LVDS connection
> + * @LVDS_DUAL_LINK_EVEN_ODD_PIXELS: Even pixels are expected to be generated
> + *    from the first port, odd pixels from the second port
> + * @LVDS_DUAL_LINK_ODD_EVEN_PIXELS: Odd pixels are expected to be generated
> + *    from the first port, even pixels from the second port
> + */
> +enum lvds_pixels_order {
> +	LVDS_DUAL_LINK_EVEN_ODD_PIXELS = BIT(0),
> +	LVDS_DUAL_LINK_ODD_EVEN_PIXELS = BIT(1),
> +};
> +
> +enum lvds_pixel {
> +	PIX_R_0		= 0x00,
> +	PIX_R_1		= 0x01,
> +	PIX_R_2		= 0x02,
> +	PIX_R_3		= 0x03,
> +	PIX_R_4		= 0x04,
> +	PIX_R_5		= 0x05,
> +	PIX_R_6		= 0x06,
> +	PIX_R_7		= 0x07,
> +	PIX_G_0		= 0x08,
> +	PIX_G_1		= 0x09,
> +	PIX_G_2		= 0x0A,
> +	PIX_G_3		= 0x0B,
> +	PIX_G_4		= 0x0C,
> +	PIX_G_5		= 0x0D,
> +	PIX_G_6		= 0x0E,
> +	PIX_G_7		= 0x0F,
> +	PIX_B_0		= 0x10,
> +	PIX_B_1		= 0x11,
> +	PIX_B_2		= 0x12,
> +	PIX_B_3		= 0x13,
> +	PIX_B_4		= 0x14,
> +	PIX_B_5		= 0x15,
> +	PIX_B_6		= 0x16,
> +	PIX_B_7		= 0x17,
> +	PIX_H_S		= 0x18,
> +	PIX_V_S		= 0x19,
> +	PIX_D_E		= 0x1A,
> +	PIX_C_E		= 0x1B,
> +	PIX_C_I		= 0x1C,
> +	PIX_TOG		= 0x1D,
> +	PIX_ONE		= 0x1E,
> +	PIX_ZER		= 0x1F,
> +};
> +
> +/*
> + * Expected JEIDA-RGB888 data to be sent in LSB format
> + *	    bit6 ............................bit0
> + */
> +const enum lvds_pixel lvds_bitmap_jeida_rgb888[5][7] = {
> +	{ PIX_ONE, PIX_ONE, PIX_ZER, PIX_ZER, PIX_ZER, PIX_ONE, PIX_ONE },
> +	{ PIX_G_2, PIX_R_7, PIX_R_6, PIX_R_5, PIX_R_4, PIX_R_3, PIX_R_2 },
> +	{ PIX_B_3, PIX_B_2, PIX_G_7, PIX_G_6, PIX_G_5, PIX_G_4, PIX_G_3 },
> +	{ PIX_D_E, PIX_V_S, PIX_H_S, PIX_B_7, PIX_B_6, PIX_B_5, PIX_B_4 },
> +	{ PIX_C_E, PIX_B_1, PIX_B_0, PIX_G_1, PIX_G_0, PIX_R_1, PIX_R_0 }
> +};
> +
> +/*
> + * Expected VESA-RGB888 data to be sent in LSB format
> + *	    bit6 ............................bit0
> + */
> +const enum lvds_pixel lvds_bitmap_vesa_rgb888[5][7] = {
> +	{ PIX_ONE, PIX_ONE, PIX_ZER, PIX_ZER, PIX_ZER, PIX_ONE, PIX_ONE },
> +	{ PIX_G_0, PIX_R_5, PIX_R_4, PIX_R_3, PIX_R_2, PIX_R_1, PIX_R_0 },
> +	{ PIX_B_1, PIX_B_0, PIX_G_5, PIX_G_4, PIX_G_3, PIX_G_2, PIX_G_1 },
> +	{ PIX_D_E, PIX_V_S, PIX_H_S, PIX_B_5, PIX_B_4, PIX_B_3, PIX_B_2 },
> +	{ PIX_C_E, PIX_B_7, PIX_B_6, PIX_G_7, PIX_G_6, PIX_R_7, PIX_R_6 }
> +};
> +
> +static inline void lvds_writel(struct stm32_lvds *lvds, u32 reg, u32 val)
> +{
> +	writel(val, lvds->base + reg);
> +}
> +
> +static inline u32 lvds_readl(struct stm32_lvds *lvds, u32 reg)
> +{
> +	return readl(lvds->base + reg);
> +}
> +
> +static inline void lvds_set(struct stm32_lvds *lvds, u32 reg, u32 mask)
> +{
> +	lvds_writel(lvds, reg, lvds_readl(lvds, reg) | mask);
> +}
> +
> +static inline void lvds_clear(struct stm32_lvds *lvds, u32 reg, u32 mask)
> +{
> +	lvds_writel(lvds, reg, lvds_readl(lvds, reg) & ~mask);
> +}
> +
> +/* Integer mode */
> +#define EN_SD		0
> +#define EN_TWG		0
> +#define DOWN_SPREAD	0
> +#define TEST_DIV	70

Put these defines with the others, above first function. 

> +
> +static u32 pll_get_clkout_khz(u32 clkin_khz, u32 bdiv, u32 mdiv, u32 ndiv)
> +{
> +	int divisor = ndiv * bdiv;
> +
> +	/* Prevents from division by 0 */
> +	if (!divisor)
> +		return 0;
> +
> +	return clkin_khz * mdiv / divisor;
> +}
> +
> +#define NDIV_MIN	2
> +#define NDIV_MAX	6
> +#define BDIV_MIN	2
> +#define BDIV_MAX	6
> +#define MDIV_MIN	1
> +#define MDIV_MAX	1023

ditto

> +
> +static int lvds_pll_get_params(u32 clkin_khz, u32 clkout_khz,
> +			       u32 *bdiv, u32 *mdiv, u32 *ndiv)
> +{
> +	u32 i, o, n;
> +	u32 delta, best_delta; /* all in khz */
> +
> +	/* Early checks preventing division by 0 & odd results */
> +	if (clkin_khz <= 0 || clkout_khz <= 0)

clkin_khz and clkout_khz are unsigned, so can't be negative. 

if (clkin_khz = 0 || clkout_khz = 0)  should be engouh 

> +		return -EINVAL;
> +
> +	best_delta = 1000000; /* big started value (1000000khz) */
> +
> +	for (i = NDIV_MIN; i <= NDIV_MAX; i++) {
> +		for (o = BDIV_MIN; o <= BDIV_MAX; o++) {
> +			n = DIV_ROUND_CLOSEST(i * o * clkout_khz, clkin_khz);
> +			/* Check ndiv according to vco range */
> +			if (n < MDIV_MIN || n > MDIV_MAX)
> +				continue;
> +			/* Check if new delta is better & saves parameters */
> +			delta = abs(pll_get_clkout_khz(clkin_khz, i, n, o) - clkout_khz);
> +			if (delta < best_delta) {
> +				*ndiv = i;
> +				*mdiv = n;
> +				*bdiv = o;
> +				best_delta = delta;
> +			}
> +			/* fast return in case of "perfect result" */
> +			if (!delta)
> +				return 0;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int stm32_lvds_pll_enable(struct stm32_lvds *lvds,
> +				 struct display_timing *timings,
> +				 int phy)
> +{
> +	u32 pll_in_khz, bdiv = 0, mdiv = 0, ndiv = 0;
> +	int ret, val, multiplier;
> +
> +	/* Release PHY from reset */
> +	lvds_set(lvds, LVDS_PxGCR(phy), PHY_GCR_DIV_RSTN | PHY_GCR_RSTZ);
> +
> +	/* lvds_pll_config */
> +	/* Set PLL Slv & Mst configs and timings */
> +	pll_in_khz = lvds->refclk / 1000;
> +
> +	if (lvds->dual_link)
> +		multiplier = 2;
> +	else
> +		multiplier = 1;
> +
> +	ret = lvds_pll_get_params(pll_in_khz, timings->pixelclock.typ * 7 / 1000 / multiplier,
> +				  &bdiv, &mdiv, &ndiv);
> +	if (ret)
> +		return ret;
> +
> +	/* Set PLL parameters */
> +	lvds_writel(lvds, LVDS_PxPLLCR2(phy), (ndiv << 16) | bdiv);
> +	lvds_writel(lvds, LVDS_PxPLLSDCR1(phy), mdiv);
> +	lvds_writel(lvds, LVDS_PxPLLTESTCR(phy), TEST_DIV << 16);
> +
> +	/* Disable TWG and SD: for now, PLL just need to be in integer mode */
> +	lvds_clear(lvds, LVDS_PxPLLCR1(phy), EN_TWG | EN_SD);
> +
> +	/* Power up bias and PLL dividers */
> +	lvds_set(lvds, LVDS_PxDCR(phy), POWER_OK);
> +
> +	lvds_set(lvds, LVDS_PxCMCR1(phy), CM_EN_DL);
> +	lvds_set(lvds, LVDS_PxCMCR2(phy), CM_EN_DL4);
> +
> +	lvds_set(lvds, LVDS_PxPLLCPCR(phy), 0x1);

Add a define to replace 0x1, for example #defince PLL_CHARGE_PUMP_CONTROL_DEFAULT 0x1 ?

> +	lvds_set(lvds, LVDS_PxBCR3(phy), VM_EN_DL);
> +	lvds_set(lvds, LVDS_PxBCR1(phy), EN_BIAS_DL);
> +	lvds_set(lvds, LVDS_PxCFGCR(phy), EN_DIG_DL);
> +
> +	/* lvds_pll_enable */
> +	/* PLL lock timing control for the monitor unmask after startup (pll_en) */
> +	/* Adjust the value so that the masking window is opened at start-up */
> +	/* MST_MON_PLL_LOCK_UNMASK_TUNE */
> +	lvds_writel(lvds, LVDS_PxMPLCR(phy), (0x200 - 0x160) << 16);

What are 0x200 and 0x160 ? add a define with a meaningful name for each of these value.

> +
> +	lvds_writel(lvds, LVDS_PxBCR2(phy), BIAS_EN);
> +
> +	lvds_set(lvds, LVDS_PxGCR(phy),
> +		 PHY_GCR_DP_CLK_OUT | PHY_GCR_LS_CLK_OUT | PHY_GCR_BIT_CLK_OUT);
> +
> +	/* TODO hardcoded values for now */
> +	lvds_set(lvds, LVDS_PxPLLTESTCR(phy), BIT(8) /* PLL_TEST_DIV_EN */);

Why hardcoded value for now ? simply add a define PLL_TEST_DIV_EN BIT(8) ?

> +	lvds_set(lvds, LVDS_PxPLLCR1(phy), BIT(8) /* PLL_DIVIDERS_ENABLE */);

ditto

> +
> +	lvds_set(lvds, LVDS_PxSCR(phy), BIT(16) /* SER_DATA_OK */);

ditto

> +
> +	/* Enable the LVDS PLL & wait for its lock */
> +	lvds_set(lvds, LVDS_PxPLLCR1(phy), PLL_EN);
> +	ret = readl_poll_sleep_timeout(lvds->base + LVDS_PxPLLSR(phy),
> +				       val, val & PLL_LOCK, SLEEP_US, TIMEOUT_US);
> +	if (ret)
> +		return ret;
> +
> +	/* Select MST PHY clock as pixel clock for the LDITX instead of FREF */
> +	/* WCLKCR_SLV_CLKPIX_SEL is for dual link */
> +	lvds_writel(lvds, LVDS_WCLKCR, WCLKCR_SLV_CLKPIX_SEL);
> +
> +	lvds_set(lvds, LVDS_PxPLLTESTCR(phy), BIT(0));

add #define PLL_TEST_CLK_EN BIT(0) instead ?

lvds_set(lvds, LVDS_PxPLLTESTCR(phy), PLL_TEST_CLK_EN);


> +
> +	return 0;
> +}
> +
> +static int stm32_lvds_enable(struct udevice *dev,
> +			     const struct display_timing *timings)
> +{
> +	struct stm32_lvds *lvds = dev_get_priv(dev);
> +	u32 lvds_cdl1cr = 0;
> +	u32 lvds_cdl2cr = 0;
> +	u32 lvds_dmlcr = 0;
> +	u32 lvds_dmmcr = 0;
> +	u32 lvds_cr = 0;
> +	int i;
> +
> +	lvds_clear(lvds, LVDS_CDL1CR, CDLCRx_DISTR0 | CDLCRx_DISTR1 | CDLCRx_DISTR2
> +					| CDLCRx_DISTR3 | CDLCRx_DISTR4);
> +	lvds_clear(lvds, LVDS_CDL2CR, CDLCRx_DISTR0 | CDLCRx_DISTR1 | CDLCRx_DISTR2
> +					| CDLCRx_DISTR3 | CDLCRx_DISTR4);
> +
> +	/* Set channel distribution */
> +	lvds_cr &= ~CR_LKMOD;
> +	lvds_cdl1cr = CDL1CR_DEFAULT;
> +
> +	if (lvds->dual_link) {
> +		lvds_cr |= CR_LKMOD;
> +		lvds_cdl2cr = CDL2CR_DEFAULT;
> +	}
> +
> +	/* Set signal polarity */
> +	if (timings->flags & DISPLAY_FLAGS_DE_LOW)
> +		lvds_cr |= CR_DEPOL;
> +
> +	if (timings->flags & DISPLAY_FLAGS_HSYNC_LOW)
> +		lvds_cr |= CR_HSPOL;
> +
> +	if (timings->flags & DISPLAY_FLAGS_VSYNC_LOW)
> +		lvds_cr |= CR_VSPOL;
> +
> +	/* Set link phase */
> +	switch (lvds->dual_link) {
> +	case LVDS_DUAL_LINK_EVEN_ODD_PIXELS: /* LKPHA = 0 */
> +		lvds_cr &= ~CR_LKPHA;
> +		break;
> +	case LVDS_DUAL_LINK_ODD_EVEN_PIXELS: /* LKPHA = 1 */
> +		lvds_cr |= CR_LKPHA;
> +		break;
> +	default:
> +		dev_dbg(dev, "No phase precised, setting default\n");
> +		lvds_cr &= ~CR_LKPHA;
> +		break;
> +	}
> +
> +	/* Set Data Mapping */
> +	switch (lvds->bus_format) {
> +	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: /* VESA-RGB888 */
> +		for (i = 0; i < 5; i++) {
> +			lvds_dmlcr = ((lvds_bitmap_vesa_rgb888[i][0])
> +				      + (lvds_bitmap_vesa_rgb888[i][1] << 5)
> +				      + (lvds_bitmap_vesa_rgb888[i][2] << 10)
> +				      + (lvds_bitmap_vesa_rgb888[i][3] << 15));
> +			lvds_dmmcr = ((lvds_bitmap_vesa_rgb888[i][4])
> +				      + (lvds_bitmap_vesa_rgb888[i][5] << 5)
> +				      + (lvds_bitmap_vesa_rgb888[i][6] << 10));
> +
> +			/* Write registers at the end of computations */
> +			lvds_writel(lvds, LVDS_DMLCR(i), lvds_dmlcr);
> +			lvds_writel(lvds, LVDS_DMMCR(i), lvds_dmmcr);
> +		}
> +		break;
> +	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: /* JEIDA-RGB888 */
> +		for (i = 0; i < 5; i++) {
> +			lvds_dmlcr = ((lvds_bitmap_jeida_rgb888[i][0])
> +				      + (lvds_bitmap_jeida_rgb888[i][1] << 5)
> +				      + (lvds_bitmap_jeida_rgb888[i][2] << 10)
> +				      + (lvds_bitmap_jeida_rgb888[i][3] << 15));
> +			lvds_dmmcr = ((lvds_bitmap_jeida_rgb888[i][4])
> +				      + (lvds_bitmap_jeida_rgb888[i][5] << 5)
> +				      + (lvds_bitmap_jeida_rgb888[i][6] << 10));
> +
> +			/* Write registers at the end of computations */
> +			lvds_writel(lvds, LVDS_DMLCR(i), lvds_dmlcr);
> +			lvds_writel(lvds, LVDS_DMMCR(i), lvds_dmmcr);
> +		}
> +		break;
> +	default:
> +		dev_dbg(dev, "Unsupported LVDS bus format 0x%04x\n", lvds->bus_format);
> +	}
> +
> +	/* Turn the output on */
> +	lvds_cr |= CR_LVDSEN;
> +
> +	/* Commit config to registers */
> +	lvds_set(lvds, LVDS_CR, lvds_cr);
> +	lvds_writel(lvds, LVDS_CDL1CR, lvds_cdl1cr);
> +	lvds_writel(lvds, LVDS_CDL2CR, lvds_cdl2cr);
> +
> +	return 0;
> +}
> +
> +static int stm32_lvds_attach(struct udevice *dev)
> +{
> +	struct stm32_lvds *lvds = dev_get_priv(dev);
> +	struct display_timing timings;
> +	int ret;
> +
> +	ret = panel_get_display_timing(lvds->panel, &timings);
> +	if (ret) {
> +		ret = ofnode_decode_display_timing(dev_ofnode(lvds->panel),
> +						   0, &timings);
> +		if (ret) {
> +			dev_err(dev, "decode display timing error %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = stm32_lvds_enable(dev, &timings);
> +
> +	return ret;
> +}
> +
> +static int stm32_lvds_set_backlight(struct udevice *dev, int percent)
> +{
> +	struct stm32_lvds *lvds = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = panel_enable_backlight(lvds->panel);
> +	if (ret) {
> +		dev_err(dev, "panel %s enable backlight error %d\n",
> +			lvds->panel->name, ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static int lvds_handle_pixel_order(struct stm32_lvds *lvds)
> +{
> +	ofnode parent, panel_port0, panel_port1;
> +	bool even_pixels, odd_pixels;
> +	int port0, port1;
> +
> +	/*
> +	 * In case we are operating in single link,
> +	 * there is only one port linked to the LVDS.
> +	 * Check whether we are in this case and exit if yes.
> +	 */
> +	parent = ofnode_find_subnode(dev_ofnode(lvds->panel), "ports");
> +	if (!ofnode_valid(parent))
> +		return 0;
> +
> +	panel_port0 = ofnode_first_subnode(parent);
> +	if (!ofnode_valid(panel_port0))
> +		return -EPIPE;
> +
> +	even_pixels = ofnode_read_bool(panel_port0, "dual-lvds-even-pixels");
> +	odd_pixels = ofnode_read_bool(panel_port0, "dual-lvds-odd-pixels");
> +	if (even_pixels && odd_pixels)
> +		return -EINVAL;
> +
> +	port0 = even_pixels ? LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
> +		LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
> +
> +	panel_port1 = ofnode_next_subnode(panel_port0);
> +	if (!ofnode_valid(panel_port1))
> +		return -EPIPE;
> +
> +	even_pixels = ofnode_read_bool(panel_port1, "dual-lvds-even-pixels");
> +	odd_pixels = ofnode_read_bool(panel_port1, "dual-lvds-odd-pixels");
> +	if (even_pixels && odd_pixels)
> +		return -EINVAL;
> +
> +	port1 = even_pixels ? LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
> +		LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
> +
> +	/*
> +	 * A valid dual-LVDS bus is found when one port is marked with
> +	 * "dual-lvds-even-pixels", and the other port is marked with
> +	 * "dual-lvds-odd-pixels", bail out if the markers are not right.
> +	 */
> +	if (port0 + port1 != LVDS_DUAL_LINK_EVEN_ODD_PIXELS + LVDS_DUAL_LINK_ODD_EVEN_PIXELS)
> +		return -EINVAL;
> +
> +	return port0;
> +}
> +
> +static int stm32_lvds_probe(struct udevice *dev)
> +{
> +	struct stm32_lvds *priv = dev_get_priv(dev);
> +	struct display_timing timings;
> +	struct reset_ctl rst;
> +	struct clk pclk, refclk;
> +	const char *data_mapping;
> +	int ret;
> +
> +	priv->base = dev_read_addr_ptr(dev);
> +	if ((fdt_addr_t)priv->base == FDT_ADDR_T_NONE) {
> +		dev_err(dev, "Unable to read LVDS base address\n");
> +		return -EINVAL;
> +	}
> +

Device tree resources reading must be located in an of_to_plat() function, not in probe().
See for example, see ./drivers/spi/stm32_spi.c

For reference : https://docs.u-boot.org/en/stable/develop/driver-model/design.html#reading-ofdata



> +	ret = clk_get_by_name(dev, "pclk", &pclk);
> +	if (ret) {
> +		dev_err(dev, "Unable to get peripheral clock: %d\n", ret);
> +		return ret;

ditto

> +	}
> +
> +	ret = clk_enable(&pclk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable peripheral clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_get_by_name(dev, "ref", &refclk);
> +	if (ret) {
> +		dev_err(dev, "Unable to get reference clock: %d\n", ret);
> +		goto err_clk;
> +	}

ditto

> +
> +	ret = clk_enable(&refclk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable reference clock: %d\n", ret);
> +		goto err_clk;
> +	}
> +
> +	priv->refclk = (unsigned int)clk_get_rate(&refclk);
> +
> +	ret = reset_get_by_index(dev, 0, &rst);
> +	if (ret) {
> +		dev_err(dev, "Failed to get LVDS reset: %d\n", ret);
> +		goto err_rst;
> +	}
> +

ditto

> +	reset_deassert(&rst);
> +
> +	ret = uclass_get_device_by_driver(UCLASS_PANEL,
> +					  DM_DRIVER_GET(simple_panel), &priv->panel);
> +	if (ret) {
> +		dev_err(dev, "panel device error %d\n", ret);
> +		goto err_rst;
> +	}

ditto
> +
> +	ret = panel_get_display_timing(priv->panel, &timings);
> +	if (ret) {
> +		ret = ofnode_decode_display_timing(dev_ofnode(priv->panel),
> +						   0, &timings);
> +		if (ret) {
> +			dev_err(dev, "decode display timing error %d\n", ret);
> +			goto err_rst;
> +		}
> +	}
> +

ditto 

> +	data_mapping = ofnode_read_string(dev_ofnode(priv->panel), "data-mapping");
> +	if (!strcmp(data_mapping, "vesa-24"))
> +		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
> +	else if (!strcmp(data_mapping, "jeida-24"))
> +		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
> +	else
> +		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;

ditto

> +
> +	/* Handle dual link config */
> +	priv->dual_link = lvds_handle_pixel_order(priv);
> +	if (priv->dual_link < 0)
> +		goto err_rst;
> +
> +	if (priv->dual_link > 0) {
> +		ret = stm32_lvds_pll_enable(priv, &timings, LVDS_PHY_SLAVE);
> +		if (ret)
> +			goto err_rst;
> +	}
> +
> +	ret = stm32_lvds_pll_enable(priv, &timings, LVDS_PHY_MASTER);
> +	if (ret)
> +		goto err_rst;
> +
> +	return 0;
> +
> +err_rst:
> +	clk_disable(&refclk);
> +err_clk:
> +	clk_disable(&pclk);
> +
> +	return ret;
> +}
> +
> +static const struct video_bridge_ops stm32_lvds_ops = {
> +	.attach = stm32_lvds_attach,
> +	.set_backlight = stm32_lvds_set_backlight,
> +};
> +
> +static const struct udevice_id stm32_lvds_ids[] = {
> +	{.compatible = "st,stm32mp25-lvds"},
> +	{}
> +};
> +
> +U_BOOT_DRIVER(stm32_lvds) = {
> +	.name		= "stm32-display-lvds",
> +	.id		= UCLASS_VIDEO_BRIDGE,
> +	.of_match	= stm32_lvds_ids,
> +	.ops		= &stm32_lvds_ops,
> +	.probe		= stm32_lvds_probe,
> +	.priv_auto	= sizeof(struct stm32_lvds),
> +};
> 

Re: [PATCH 4/8] video: stm32: ltdc: support new hardware version for STM32MP25 SoC

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> STM32MP2 SoCs feature a new version of the LTDC IP.  This new version
> features a bus clock, as well as a 150MHz pad frequency.  Add its
> compatible to the list of device to probe and handle quirks.  The new
> hardware version features a bus clock.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  drivers/video/stm32/stm32_ltdc.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/stm32/stm32_ltdc.c b/drivers/video/stm32/stm32_ltdc.c
> index 0a062c8939dbe49b11aa50f5ca9701bdbe5c5b0b..d9ba5c4ef53558fe8b5565d47b022699d9e4ea0e 100644
> --- a/drivers/video/stm32/stm32_ltdc.c
> +++ b/drivers/video/stm32/stm32_ltdc.c
> @@ -262,6 +262,7 @@ static const u32 layer_regs_a2[] = {
>  #define HWVER_10300 0x010300
>  #define HWVER_20101 0x020101
>  #define HWVER_40100 0x040100
> +#define HWVER_40101 0x040101
>  
>  enum stm32_ltdc_pix_fmt {
>  	PF_ARGB8888 = 0,	/* ARGB [32 bits] */
> @@ -529,7 +530,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  	struct udevice *bridge = NULL;
>  	struct udevice *panel = NULL;
>  	struct display_timing timings;
> -	struct clk pclk;
> +	struct clk pclk, bclk;
>  	struct reset_ctl rst;
>  	ulong rate;
>  	int ret;
> @@ -540,7 +541,21 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  		return -EINVAL;
>  	}
>  
> -	ret = clk_get_by_index(dev, 0, &pclk);
> +	ret = clk_get_by_name(dev, "bus", &bclk);
> +	if (ret) {
> +		if (ret != -ENODATA) {
> +			dev_err(dev, "bus clock get error %d\n", ret);
> +			return ret;
> +		}
> +	} else {
> +		ret = clk_enable(&bclk);
> +		if (ret) {
> +			dev_err(dev, "bus clock enable error %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_get_by_name(dev, "lcd", &pclk);
>  	if (ret) {
>  		dev_err(dev, "peripheral clock get error %d\n", ret);
>  		return ret;
> @@ -566,6 +581,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  		priv->pix_fmt_hw = pix_fmt_a1;
>  		break;
>  	case HWVER_40100:
> +	case HWVER_40101:
>  		priv->layer_regs = layer_regs_a2;
>  		priv->pix_fmt_hw = pix_fmt_a2;
>  		break;
> @@ -688,6 +704,7 @@ static int stm32_ltdc_bind(struct udevice *dev)
>  
>  static const struct udevice_id stm32_ltdc_ids[] = {
>  	{ .compatible = "st,stm32-ltdc" },
> +	{ .compatible = "st,stm32mp251-ltdc" },
>  	{ }
>  };
>  
> 
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

Re: [PATCH 5/8] video: stm32: ltdc: properly search the first available panel

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> Initially there was only one DSI bridge with one panel attached to this
> device. This explained the call to uclass_first_device_err(UCLASS_PANEL,
> ...) which worked fine at the time.
> 
> Now that multiple bridges and panels, with different technologies, can
> be plugged onto the board this way to get the panel device is outdated.
> 
> The lookup is done is two steps. First we cirle through the

typo /cirle/circle 

With this typo fixed, you add my Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

> UCLASS_VIDEO_BRIDGE, and once we get one, we search through its
> endpoints until we get a UCLASS_PANEL device available.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  drivers/video/stm32/stm32_ltdc.c | 136 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 125 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/video/stm32/stm32_ltdc.c b/drivers/video/stm32/stm32_ltdc.c
> index d9ba5c4ef53558fe8b5565d47b022699d9e4ea0e..cdcfde678fb59ad884125e7c26f43710d76ba246 100644
> --- a/drivers/video/stm32/stm32_ltdc.c
> +++ b/drivers/video/stm32/stm32_ltdc.c
> @@ -17,6 +17,7 @@
>  #include <video_bridge.h>
>  #include <asm/io.h>
>  #include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
>  #include <dm/device_compat.h>
>  #include <linux/bitops.h>
>  #include <linux/printk.h>
> @@ -495,6 +496,101 @@ static void stm32_ltdc_set_layer1(struct stm32_ltdc_priv *priv, ulong fb_addr)
>  	setbits_le32(priv->regs + LTDC_L1CR, LXCR_LEN);
>  }
>  
> +static int stm32_ltdc_get_remote_device(struct udevice *dev, ofnode ep_node,
> +					enum uclass_id id, struct udevice **remote_dev)
> +{
> +	u32 remote_phandle;
> +	ofnode remote;
> +	int ret = 0;
> +
> +	ret = ofnode_read_u32(ep_node, "remote-endpoint", &remote_phandle);
> +	if (ret) {
> +		dev_err(dev, "%s(%s): Could not find remote-endpoint property\n",
> +			__func__, dev_read_name(dev));
> +		return ret;
> +	}
> +
> +	remote = ofnode_get_by_phandle(remote_phandle);
> +	if (!ofnode_valid(remote))
> +		return -EINVAL;
> +
> +	while (ofnode_valid(remote)) {
> +		remote = ofnode_get_parent(remote);
> +		if (!ofnode_valid(remote)) {
> +			dev_dbg(dev, "%s(%s): no uclass_id %d for remote-endpoint\n",
> +				__func__, dev_read_name(dev), id);
> +			continue;
> +		}
> +
> +		ret = uclass_find_device_by_ofnode(id, remote, remote_dev);
> +		if (*remote_dev && !ret) {
> +			ret = uclass_get_device_by_ofnode(id, remote, remote_dev);
> +			if (ret)
> +				dev_dbg(dev, "%s(%s): failed to get remote device %s\n",
> +					__func__, dev_read_name(dev), dev_read_name(*remote_dev));
> +			break;
> +		}
> +	};
> +
> +	return ret;
> +}
> +
> +static int stm32_ltdc_get_panel(struct udevice *dev, struct udevice **panel)
> +{
> +	ofnode ep_node, node, ports;
> +	int ret = 0;
> +
> +	if (!dev)
> +		return -EINVAL;
> +
> +	ports = ofnode_find_subnode(dev_ofnode(dev), "ports");
> +	if (!ofnode_valid(ports)) {
> +		dev_err(dev, "Remote bridge subnode\n");
> +		return ret;
> +	}
> +
> +	for (node = ofnode_first_subnode(ports);
> +	     ofnode_valid(node);
> +	     node = dev_read_next_subnode(node)) {
> +		ep_node = ofnode_first_subnode(node);
> +		if (!ofnode_valid(ep_node))
> +			continue;
> +
> +		ret = stm32_ltdc_get_remote_device(dev, ep_node, UCLASS_PANEL, panel);
> +	}
> +
> +	/* Sanity check, we can get out of the loop without having a clean ofnode */
> +	if (!(*panel))
> +		ret = -EINVAL;
> +	else
> +		if (!ofnode_valid(dev_ofnode(*panel)))
> +			ret = -EINVAL;
> +
> +	return ret;
> +}
> +
> +static int stm32_ltdc_display_init(struct udevice *dev, ofnode *ep_node,
> +				   struct udevice **panel, struct udevice **bridge)
> +{
> +	int ret;
> +
> +	if (*panel)
> +		return -EINVAL;
> +
> +	if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
> +		ret = stm32_ltdc_get_remote_device(dev, *ep_node, UCLASS_VIDEO_BRIDGE, bridge);
> +		if (ret)
> +			return ret;
> +
> +		ret = stm32_ltdc_get_panel(*bridge, panel);
> +	} else {
> +		/* no bridge, search a panel from display controller node */
> +		ret = stm32_ltdc_get_remote_device(dev, *ep_node, UCLASS_PANEL, panel);
> +	}
> +
> +	return ret;
> +}
> +
>  #if IS_ENABLED(CONFIG_TARGET_STM32F469_DISCOVERY)
>  static int stm32_ltdc_alloc_fb(struct udevice *dev)
>  {
> @@ -532,6 +628,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  	struct display_timing timings;
>  	struct clk pclk, bclk;
>  	struct reset_ctl rst;
> +	ofnode node, port;
>  	ulong rate;
>  	int ret;
>  
> @@ -568,7 +665,7 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  	}
>  
>  	priv->hw_version = readl(priv->regs + LTDC_IDR);
> -	debug("%s: LTDC hardware 0x%x\n", __func__, priv->hw_version);
> +	dev_dbg(dev, "%s: LTDC hardware 0x%x\n", __func__, priv->hw_version);
>  
>  	switch (priv->hw_version) {
>  	case HWVER_10200:
> @@ -589,13 +686,35 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  		return -ENODEV;
>  	}
>  
> -	ret = uclass_first_device_err(UCLASS_PANEL, &panel);
> -	if (ret) {
> -		if (ret != -ENODEV)
> -			dev_err(dev, "panel device error %d\n", ret);
> -		return ret;
> +	/*
> +	 * Try all the ports until one working.
> +	 *
> +	 * This is done in two times. First is checks for the
> +	 * UCLASS_VIDEO_BRIDGE available, and then for this bridge
> +	 * it scans for a UCLASS_PANEL.
> +	 */
> +
> +	port = dev_read_subnode(dev, "port");
> +	if (!ofnode_valid(port)) {
> +		dev_err(dev, "%s(%s): 'port' subnode not found\n",
> +			__func__, dev_read_name(dev));
> +		return -EINVAL;
>  	}
>  
> +	for (node = ofnode_first_subnode(port);
> +	     ofnode_valid(node);
> +	     node = dev_read_next_subnode(node)) {
> +		ret = stm32_ltdc_display_init(dev, &node, &panel, &bridge);
> +		if (ret)
> +			dev_dbg(dev, "Device failed ret=%d\n", ret);
> +		else
> +			break;
> +	}
> +
> +	/* Sanity check */
> +	if (ret)
> +		return ret;
> +
>  	ret = panel_get_display_timing(panel, &timings);
>  	if (ret) {
>  		ret = ofnode_decode_display_timing(dev_ofnode(panel),
> @@ -624,11 +743,6 @@ static int stm32_ltdc_probe(struct udevice *dev)
>  	reset_deassert(&rst);
>  
>  	if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
> -		ret = uclass_get_device(UCLASS_VIDEO_BRIDGE, 0, &bridge);
> -		if (ret)
> -			dev_dbg(dev,
> -				"No video bridge, or no backlight on bridge\n");
> -
>  		if (bridge) {
>  			ret = video_bridge_attach(bridge);
>  			if (ret) {
> 

Re: [PATCH 6/8] ARM: dts: stm32: use LTDC and LVDS nodes before relocation in stm32mp25-u-boot

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> Use LTDC and LVDS nodes in all boot phases.  This is specially useful
> before U-Boot relocation.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  arch/arm/dts/stm32mp25-u-boot.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/dts/stm32mp25-u-boot.dtsi b/arch/arm/dts/stm32mp25-u-boot.dtsi
> index d9aeeb6d510df9dea9700148cf8a8ad5efcfd4f9..3ac35c4a6bc27c72eeeb532742fd4412b7aa1b85 100644
> --- a/arch/arm/dts/stm32mp25-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp25-u-boot.dtsi
> @@ -93,6 +93,14 @@
>  	bootph-all;
>  };
>  
> +&ltdc {
> +	bootph-all;
> +};
> +
> +&lvds {
> +	bootph-all;
> +};
> +
>  &pinctrl {
>  	bootph-all;
>  };
> 

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

Re: [PATCH 8/8] configs: stm32mp25: enable LVDS display support

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> Compile VIDEO_STM32 and VIDEO_STM32_LVDS by default.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  configs/stm32mp25_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/configs/stm32mp25_defconfig b/configs/stm32mp25_defconfig
> index 14619ffd96c71028b508897c77f9c8a747166683..bb9bb935e2d427fc515b83fb32ed93cdcac34c1d 100644
> --- a/configs/stm32mp25_defconfig
> +++ b/configs/stm32mp25_defconfig
> @@ -76,6 +76,9 @@ CONFIG_SERIAL_RX_BUFFER=y
>  CONFIG_SPI=y
>  CONFIG_DM_SPI=y
>  # CONFIG_OPTEE_TA_AVB is not set
> +CONFIG_VIDEO=y
> +CONFIG_VIDEO_STM32=y
> +CONFIG_VIDEO_STM32_LVDS=y
>  CONFIG_WDT=y
>  CONFIG_WDT_STM32MP=y
>  CONFIG_WDT_ARM_SMC=y
> 
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

Re: [PATCH 7/8] ARM: dts: stm32: remove panel property in stm32mp257f-ev1-u-boot

[Patrice CHOTARD]

On 8/20/25 18:17, Raphael Gallais-Pou wrote:
> SCMI regulators are not yet implemented in OP-TEE.  This leads to an
> invalid argument error when retrieving the 'power-supply' property.
> Ultimately this chains into the panel not probing and makes the whole
> display pipe fail.
> 
> Remove the property in the upstream device-tree to allow display pipe to
> boot.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
> index 9a566e18d3fd9b0cf4e21605f2cb060b49fadcb6..7a4b623664a1e167ccc0f4c73a979a083337d7a2 100644
> --- a/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
> @@ -12,6 +12,10 @@
>  	};
>  };
>  
> +&panel_lvds {
> +	/delete-property/ power-supply;
> +};
> +
>  &usart2 {
>  	bootph-all;
>  };
> 

This patch is a temporary patch right ?
If yes, i am not in favor to applied this patch, i prefer to see that there is something missing.

Thanks

Re: [PATCH 3/8] video: stm32: STM32 driver support for LVDS

[Raphael Gallais-Pou]

On 8/25/25 11:42, Patrice CHOTARD wrote:
>
> On 8/20/25 18:17, Raphael Gallais-Pou wrote:
>> The LVDS Display Interface Transmitter handles the LVDS protocol:
>> it maps the pixels received from the upstream Pixel-DMA (LTDC)
>> onto the LVDS PHY.
>>
>> The LVDS controller driver supports the following high-level features:
>>         • FDP-Link-I and OpenLDI (v0.95) protocols
>>         • Single-Link or Dual-Link operation
>>         • Single-Display or Double-Display (with the same content
>>           duplicated on both)
>>         • Flexible Bit-Mapping, including JEIDA and VESA
>>         • RGB888 or RGB666 output
>>         • Synchronous design, with one input pixel per clock cycle
>>         • No resolution limitation.
>>
>> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
>> ---
>>  MAINTAINERS                      |   1 +
>>  doc/board/st/st-dt.rst           |   1 +
>>  drivers/video/stm32/Kconfig      |   9 +
>>  drivers/video/stm32/Makefile     |   1 +
>>  drivers/video/stm32/stm32_lvds.c | 673 +++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 685 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3fb163aa1db1dba249e60b40ab3e785db4fde5d3..604ba4ca04c1079b14cd8f4c1fff4c12da0e278d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -728,6 +728,7 @@ F:	drivers/serial/serial_stm32.*
>>  F:	drivers/spi/stm32_qspi.c
>>  F:	drivers/spi/stm32_spi.c
>>  F:	drivers/video/stm32/stm32_ltdc.c
>> +F:	drivers/video/stm32/stm32_lvds.c
>>  F:	drivers/watchdog/stm32mp_wdt.c
>>  F:	include/dt-bindings/clock/stm32fx-clock.h
>>  F:	include/dt-bindings/clock/stm32mp*
>> diff --git a/doc/board/st/st-dt.rst b/doc/board/st/st-dt.rst
>> index 2a285c81807ee26924948d8c2497f11dfe45b6e6..28cada97e72da711408a1aaaa928ad3faa51162e 100644
>> --- a/doc/board/st/st-dt.rst
>> +++ b/doc/board/st/st-dt.rst
>> @@ -25,6 +25,7 @@ kernel binding directory = Documentation/devicetree/bindings/
>>  * display
>>  	- display/st,stm32-dsi.yaml
>>  	- display/st,stm32-ltdc.yaml
>> +        - display/st,stm32mp25-lvds.yaml
>>  * gpio
>>  	- pinctrl/st,stm32-pinctrl.yaml
>>  * hwlock
>> diff --git a/drivers/video/stm32/Kconfig b/drivers/video/stm32/Kconfig
>> index c354c402c288b4c004bd2b5cc74a4d00beef2773..4cb8a841caf51aaba5115b9fcddb967f73a3d9b4 100644
>> --- a/drivers/video/stm32/Kconfig
>> +++ b/drivers/video/stm32/Kconfig
>> @@ -23,6 +23,15 @@ config VIDEO_STM32_DSI
>>  	  This option enables support DSI internal bridge which can be used on
>>  	  devices which have DSI devices connected.
>>  
>> +config VIDEO_STM32_LVDS
>> +	bool "Enable STM32 LVDS video support"
>> +	depends on VIDEO_STM32
>> +	select VIDEO_BRIDGE
>> +	select VIDEO_DW_MIPI_DSI
>> +	help
>> +	  This enables Low Voltage Differential Signaling (LVDS) display
>> +	  support.
>> +
>>  config VIDEO_STM32_MAX_XRES
>>  	int "Maximum horizontal resolution (for memory allocation purposes)"
>>  	depends on VIDEO_STM32
>> diff --git a/drivers/video/stm32/Makefile b/drivers/video/stm32/Makefile
>> index f8b42d1a4d126a31aa11ce820e829b133ca3bb2d..059d9000c1d7762d06bff36d5408143a34848227 100644
>> --- a/drivers/video/stm32/Makefile
>> +++ b/drivers/video/stm32/Makefile
>> @@ -7,3 +7,4 @@
>>  
>>  obj-${CONFIG_VIDEO_STM32} = stm32_ltdc.o
>>  obj-${CONFIG_VIDEO_STM32_DSI} += stm32_dsi.o
>> +obj-${CONFIG_VIDEO_STM32_LVDS} += stm32_lvds.o
>> diff --git a/drivers/video/stm32/stm32_lvds.c b/drivers/video/stm32/stm32_lvds.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..1e3e740122dfee0c947819532da64f0d99327bde
>> --- /dev/null
>> +++ b/drivers/video/stm32/stm32_lvds.c
>> @@ -0,0 +1,673 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2025 STMicroelectronics - All Rights Reserved
>> + * Author(s): Raphaël Gallais-Pou <raphael.gallais-pou@foss.st.com> for STMicroelectronics.
>> + *
>> + * This Low Voltage Differential Signal controller driver is based on the Linux Kernel driver from
>> + * drivers/gpu/drm/stm/ltdc.c
>> + */
>> +
>> +#define LOG_CATEGORY UCLASS_VIDEO_BRIDGE
>> +
>> +#include <clk.h>
>> +#include <dm.h>
>> +#include <log.h>
>> +#include <media_bus_format.h>
>> +#include <panel.h>
>> +#include <reset.h>
>> +#include <video.h>
>> +#include <video_bridge.h>
>> +#include <asm/io.h>
>> +#include <dm/device_compat.h>
>> +#include <dm/ofnode.h>
>> +#include <linux/iopoll.h>
>> +
>> +/* LVDS Host registers */
>> +#define LVDS_CR		0x0000  /* configuration register */
>> +#define LVDS_DMLCR0	0x0004  /* data mapping lsb configuration register 0	*/
>> +#define LVDS_DMMCR0	0x0008  /* data mapping msb configuration register 0	*/
>> +#define LVDS_DMLCR1	0x000C  /* data mapping lsb configuration register 1	*/
>> +#define LVDS_DMMCR1	0x0010  /* data mapping msb configuration register 1	*/
>> +#define LVDS_DMLCR2	0x0014  /* data mapping lsb configuration register 2	*/
>> +#define LVDS_DMMCR2	0x0018  /* data mapping msb configuration register 2	*/
>> +#define LVDS_DMLCR3	0x001C  /* data mapping lsb configuration register 3	*/
>> +#define LVDS_DMMCR3	0x0020  /* data mapping msb configuration register 3	*/
>> +#define LVDS_DMLCR4	0x0024  /* data mapping lsb configuration register 4	*/
>> +#define LVDS_DMMCR4	0x0028  /* data mapping msb configuration register 4	*/
>> +#define LVDS_DMLCR(id)	(LVDS_DMLCR0 + 8U * (id))
>> +#define LVDS_DMMCR(id)	(LVDS_DMMCR0 + 8U * (id))
>> +#define LVDS_CDL1CR	0x002C  /* channel distrib link 1 configuration register	*/
>> +#define LVDS_CDL2CR	0x0030  /* channel distrib link 2 configuration register	*/
>> +
>> +#define CDL1CR_DEFAULT	0x4321
>> +#define CDL2CR_DEFAULT	0x59876
>> +
>> +/* LVDS Host registers */
>> +#define LVDS_PHY_MASTER	0x0
>> +#define LVDS_PHY_SLAVE	0x100
>> +
>> +/* phy parameter can only be one of those two above */
>> +#define LVDS_PxGCR(phy)		((phy) + 0x1000)   /* Global Control Register	*/
> All define must be in uppercase, you are mixing lower/upper case which is not encouraged.
> Same remark below .

Naming is hard. :)

The LVDS has two PHYs, with each the same register names. Initially the
lowercase character was to highlight the PHY number of the LVDS. So for PHY 1 it
would be LVDS_P1CMCR1 and PHY 2 LVDS_P2CMCR1. Anyway, even in uppercase this is
the closest naming to the hardware spec, so I'll stick to this.
>
>> +#define LVDS_PxCMCR1(phy)	((phy) + 0x100C)   /* Current Mode Control Register 1 */
>> +#define LVDS_PxCMCR2(phy)	((phy) + 0x1010)  /* Current Mode Control Register 2 */
>> +#define LVDS_PxSCR(phy)		((phy) + 0x1020)  /* Serial Control Register	*/
>> +#define LVDS_PxBCR1(phy)	((phy) + 0x102C)  /* Bias Control Register 1	*/
>> +#define LVDS_PxBCR2(phy)	((phy) + 0x1030)  /* Bias Control Register 2	*/
>> +#define LVDS_PxBCR3(phy)	((phy) + 0x1034)  /* Bias Control Register 3	*/
>> +#define LVDS_PxMPLCR(phy)	((phy) + 0x1064)  /* Monitor PLL Lock Control Register */
>> +#define LVDS_PxDCR(phy)		((phy) + 0x1084)  /* Debug Control Register	*/
>> +#define LVDS_PxSSR1(phy)	((phy) + 0x1088)  /* Spare Status Register 1	*/
>> +#define LVDS_PxCFGCR(phy)	((phy) + 0x10A0)  /* Configuration Control Register */
>> +#define LVDS_PxPLLCR1(phy)	((phy) + 0x10C0)  /* PLL_MODE 1 Control Register	*/
>> +#define LVDS_PxPLLCR2(phy)	((phy) + 0x10C4)  /* PLL_MODE 2 Control Register	*/
>> +#define LVDS_PxPLLSR(phy)	((phy) + 0x10C8)  /* PLL Status Register	*/
>> +#define LVDS_PxPLLSDCR1(phy)	((phy) + 0x10CC)  /* PLL_SD_1 Control Register	*/
>> +#define LVDS_PxPLLSDCR2(phy)	((phy) + 0x10D0)  /* PLL_SD_2 Control Register	*/
>> +#define LVDS_PxPLLTWGCR1(phy)	((phy) + 0x10D4)  /* PLL_TWG_1 Control Register	*/
>> +#define LVDS_PxPLLTWGCR2(phy)	((phy) + 0x10D8)  /* PLL_TWG_2 Control Register	*/
>> +#define LVDS_PxPLLCPCR(phy)	((phy) + 0x10E0)  /* PLL_CP Control Register	*/
>> +#define LVDS_PxPLLTESTCR(phy)	((phy) + 0x10E8)  /* PLL_TEST Control Register	*/
>> +
>> +/* LVDS Wrapper registers */
>> +#define LVDS_WCLKCR	0x11B0  /* Wrapper clock control register */
>> +#define LVDS_HWCFGR	0x1FF0  /* HW configuration register	*/
>> +#define LVDS_VERR	0x1FF4  /* Version register	*/
>> +#define LVDS_IPIDR	0x1FF8  /* Identification register	*/
>> +#define LVDS_SIDR	0x1FFC  /* Size Identification register	*/
>> +
>> +#define CR_LVDSEN	BIT(0)  /* LVDS PHY Enable */
>> +#define CR_HSPOL	BIT(1)  /* HS Polarity (horizontal sync) */
>> +#define CR_VSPOL	BIT(2)  /* VS Polarity (vertical sync) */
>> +#define CR_DEPOL	BIT(3)  /* DE Polarity (data enable) */
>> +#define CR_CI		BIT(4)  /* Control Internal (software controlled bit) */
>> +#define CR_LKMOD	BIT(5)  /* Link Mode, for both Links */
>> +#define CR_LKPHA	BIT(6)  /* Link Phase, for both Links */
>> +#define CR_LK1POL	GENMASK(20, 16)  /* Link-1 output Polarity */
>> +#define CR_LK2POL	GENMASK(25, 21)  /* Link-2 output Polarity */
>> +
>> +#define DMMCRx_MAP0	GENMASK(4, 0)
>> +#define DMMCRx_MAP1	GENMASK(9, 5)
>> +#define DMMCRx_MAP2	GENMASK(14, 10)
>> +#define DMMCRx_MAP3	GENMASK(19, 15)
>> +#define DMLCRx_MAP4	GENMASK(4, 0)
>> +#define DMLCRx_MAP5	GENMASK(9, 5)
>> +#define DMLCRx_MAP6	GENMASK(14, 10)
>> +
>> +#define CDLCRx_DISTR0	GENMASK(3, 0)
>> +#define CDLCRx_DISTR1	GENMASK(7, 4)
>> +#define CDLCRx_DISTR2	GENMASK(11, 8)
>> +#define CDLCRx_DISTR3	GENMASK(15, 12)
>> +#define CDLCRx_DISTR4	GENMASK(19, 16)
>> +
>> +#define FREF_INDEX	0
>> +#define NDIV_INDEX	1
>> +#define FPFD_INDEX	2
>> +#define MDIV_INDEX	3
>> +#define FVCO_INDEX	4
>> +#define BDIV_INDEX	5
>> +#define FBIT_INDEX	6
>> +#define FLS_INDEX	7
>> +#define FDP_INDEX	8
>> +
>> +#define PHY_GCR_BIT_CLK_OUT	BIT(0)
>> +#define PHY_GCR_LS_CLK_OUT	BIT(4)
>> +#define PHY_GCR_DP_CLK_OUT	BIT(8)
>> +#define PHY_GCR_RSTZ		BIT(24)
>> +#define PHY_GCR_DIV_RSTN	BIT(25)
>> +
>> +#define PHY_PxPLLTESTCR_TDIV	GENMASK(25, 16)
>> +#define PHY_PxPLLCR2_NDIV	GENMASK(25, 16)
>> +#define PHY_PxPLLCR2_BDIV	GENMASK(9, 0)
>> +#define PHY_PxPLLSDCR1_MDIV	GENMASK(9, 0)
>> +
>> +#define PLL_EN		BIT(0)
>> +#define PLL_LOCK	BIT(0)
>> +#define CM_EN_DL	(BIT(28) | BIT(20) | BIT(12) | BIT(4))
>> +#define CM_EN_DL4	BIT(4)
>> +#define VM_EN_DL	(BIT(16) | BIT(12) | BIT(8) | BIT(4) | BIT(0))
>> +#define EN_BIAS_DL	(BIT(16) | BIT(12) | BIT(8) | BIT(4) | BIT(0))
>> +#define EN_DIG_DL	GENMASK(4, 0)
>> +#define BIAS_EN		BIT(28)
>> +#define POWER_OK	BIT(12)
>> +
>> +#define WCLKCR_SLV_CLKPIX_SEL	BIT(0)
>> +#define WCLKCR_SRCSEL		BIT(8)
>> +
>> +/* Sleep & timeout for pll lock/unlock */
>> +#define SLEEP_US	1000
>> +#define TIMEOUT_US	20000000
>> +
>> +#define PHY_SLV_OFS	0x100
>> +
>> +struct stm32_lvds {
>> +	void __iomem *base;
>> +	struct udevice *panel;
>> +	u32 refclk;
>> +	int dual_link;
>> +	int bus_format;
>> +};
>> +
>> +/*
>> + * enum lvds_pixels_order - Pixel order of an LVDS connection
>> + * @LVDS_DUAL_LINK_EVEN_ODD_PIXELS: Even pixels are expected to be generated
>> + *    from the first port, odd pixels from the second port
>> + * @LVDS_DUAL_LINK_ODD_EVEN_PIXELS: Odd pixels are expected to be generated
>> + *    from the first port, even pixels from the second port
>> + */
>> +enum lvds_pixels_order {
>> +	LVDS_DUAL_LINK_EVEN_ODD_PIXELS = BIT(0),
>> +	LVDS_DUAL_LINK_ODD_EVEN_PIXELS = BIT(1),
>> +};
>> +
>> +enum lvds_pixel {
>> +	PIX_R_0		= 0x00,
>> +	PIX_R_1		= 0x01,
>> +	PIX_R_2		= 0x02,
>> +	PIX_R_3		= 0x03,
>> +	PIX_R_4		= 0x04,
>> +	PIX_R_5		= 0x05,
>> +	PIX_R_6		= 0x06,
>> +	PIX_R_7		= 0x07,
>> +	PIX_G_0		= 0x08,
>> +	PIX_G_1		= 0x09,
>> +	PIX_G_2		= 0x0A,
>> +	PIX_G_3		= 0x0B,
>> +	PIX_G_4		= 0x0C,
>> +	PIX_G_5		= 0x0D,
>> +	PIX_G_6		= 0x0E,
>> +	PIX_G_7		= 0x0F,
>> +	PIX_B_0		= 0x10,
>> +	PIX_B_1		= 0x11,
>> +	PIX_B_2		= 0x12,
>> +	PIX_B_3		= 0x13,
>> +	PIX_B_4		= 0x14,
>> +	PIX_B_5		= 0x15,
>> +	PIX_B_6		= 0x16,
>> +	PIX_B_7		= 0x17,
>> +	PIX_H_S		= 0x18,
>> +	PIX_V_S		= 0x19,
>> +	PIX_D_E		= 0x1A,
>> +	PIX_C_E		= 0x1B,
>> +	PIX_C_I		= 0x1C,
>> +	PIX_TOG		= 0x1D,
>> +	PIX_ONE		= 0x1E,
>> +	PIX_ZER		= 0x1F,
>> +};
>> +
>> +/*
>> + * Expected JEIDA-RGB888 data to be sent in LSB format
>> + *	    bit6 ............................bit0
>> + */
>> +const enum lvds_pixel lvds_bitmap_jeida_rgb888[5][7] = {
>> +	{ PIX_ONE, PIX_ONE, PIX_ZER, PIX_ZER, PIX_ZER, PIX_ONE, PIX_ONE },
>> +	{ PIX_G_2, PIX_R_7, PIX_R_6, PIX_R_5, PIX_R_4, PIX_R_3, PIX_R_2 },
>> +	{ PIX_B_3, PIX_B_2, PIX_G_7, PIX_G_6, PIX_G_5, PIX_G_4, PIX_G_3 },
>> +	{ PIX_D_E, PIX_V_S, PIX_H_S, PIX_B_7, PIX_B_6, PIX_B_5, PIX_B_4 },
>> +	{ PIX_C_E, PIX_B_1, PIX_B_0, PIX_G_1, PIX_G_0, PIX_R_1, PIX_R_0 }
>> +};
>> +
>> +/*
>> + * Expected VESA-RGB888 data to be sent in LSB format
>> + *	    bit6 ............................bit0
>> + */
>> +const enum lvds_pixel lvds_bitmap_vesa_rgb888[5][7] = {
>> +	{ PIX_ONE, PIX_ONE, PIX_ZER, PIX_ZER, PIX_ZER, PIX_ONE, PIX_ONE },
>> +	{ PIX_G_0, PIX_R_5, PIX_R_4, PIX_R_3, PIX_R_2, PIX_R_1, PIX_R_0 },
>> +	{ PIX_B_1, PIX_B_0, PIX_G_5, PIX_G_4, PIX_G_3, PIX_G_2, PIX_G_1 },
>> +	{ PIX_D_E, PIX_V_S, PIX_H_S, PIX_B_5, PIX_B_4, PIX_B_3, PIX_B_2 },
>> +	{ PIX_C_E, PIX_B_7, PIX_B_6, PIX_G_7, PIX_G_6, PIX_R_7, PIX_R_6 }
>> +};
>> +
>> +static inline void lvds_writel(struct stm32_lvds *lvds, u32 reg, u32 val)
>> +{
>> +	writel(val, lvds->base + reg);
>> +}
>> +
>> +static inline u32 lvds_readl(struct stm32_lvds *lvds, u32 reg)
>> +{
>> +	return readl(lvds->base + reg);
>> +}
>> +
>> +static inline void lvds_set(struct stm32_lvds *lvds, u32 reg, u32 mask)
>> +{
>> +	lvds_writel(lvds, reg, lvds_readl(lvds, reg) | mask);
>> +}
>> +
>> +static inline void lvds_clear(struct stm32_lvds *lvds, u32 reg, u32 mask)
>> +{
>> +	lvds_writel(lvds, reg, lvds_readl(lvds, reg) & ~mask);
>> +}
>> +
>> +/* Integer mode */
>> +#define EN_SD		0
>> +#define EN_TWG		0
>> +#define DOWN_SPREAD	0
>> +#define TEST_DIV	70
> Put these defines with the others, above first function. 
Ack.
>
>> +
>> +static u32 pll_get_clkout_khz(u32 clkin_khz, u32 bdiv, u32 mdiv, u32 ndiv)
>> +{
>> +	int divisor = ndiv * bdiv;
>> +
>> +	/* Prevents from division by 0 */
>> +	if (!divisor)
>> +		return 0;
>> +
>> +	return clkin_khz * mdiv / divisor;
>> +}
>> +
>> +#define NDIV_MIN	2
>> +#define NDIV_MAX	6
>> +#define BDIV_MIN	2
>> +#define BDIV_MAX	6
>> +#define MDIV_MIN	1
>> +#define MDIV_MAX	1023
> ditto
Ack.
>> +
>> +static int lvds_pll_get_params(u32 clkin_khz, u32 clkout_khz,
>> +			       u32 *bdiv, u32 *mdiv, u32 *ndiv)
>> +{
>> +	u32 i, o, n;
>> +	u32 delta, best_delta; /* all in khz */
>> +
>> +	/* Early checks preventing division by 0 & odd results */
>> +	if (clkin_khz <= 0 || clkout_khz <= 0)
> clkin_khz and clkout_khz are unsigned, so can't be negative. 
>
> if (clkin_khz = 0 || clkout_khz = 0)  should be engouh 
Ack.
>
>> +		return -EINVAL;
>> +
>> +	best_delta = 1000000; /* big started value (1000000khz) */
>> +
>> +	for (i = NDIV_MIN; i <= NDIV_MAX; i++) {
>> +		for (o = BDIV_MIN; o <= BDIV_MAX; o++) {
>> +			n = DIV_ROUND_CLOSEST(i * o * clkout_khz, clkin_khz);
>> +			/* Check ndiv according to vco range */
>> +			if (n < MDIV_MIN || n > MDIV_MAX)
>> +				continue;
>> +			/* Check if new delta is better & saves parameters */
>> +			delta = abs(pll_get_clkout_khz(clkin_khz, i, n, o) - clkout_khz);
>> +			if (delta < best_delta) {
>> +				*ndiv = i;
>> +				*mdiv = n;
>> +				*bdiv = o;
>> +				best_delta = delta;
>> +			}
>> +			/* fast return in case of "perfect result" */
>> +			if (!delta)
>> +				return 0;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_lvds_pll_enable(struct stm32_lvds *lvds,
>> +				 struct display_timing *timings,
>> +				 int phy)
>> +{
>> +	u32 pll_in_khz, bdiv = 0, mdiv = 0, ndiv = 0;
>> +	int ret, val, multiplier;
>> +
>> +	/* Release PHY from reset */
>> +	lvds_set(lvds, LVDS_PxGCR(phy), PHY_GCR_DIV_RSTN | PHY_GCR_RSTZ);
>> +
>> +	/* lvds_pll_config */
>> +	/* Set PLL Slv & Mst configs and timings */
>> +	pll_in_khz = lvds->refclk / 1000;
>> +
>> +	if (lvds->dual_link)
>> +		multiplier = 2;
>> +	else
>> +		multiplier = 1;
>> +
>> +	ret = lvds_pll_get_params(pll_in_khz, timings->pixelclock.typ * 7 / 1000 / multiplier,
>> +				  &bdiv, &mdiv, &ndiv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Set PLL parameters */
>> +	lvds_writel(lvds, LVDS_PxPLLCR2(phy), (ndiv << 16) | bdiv);
>> +	lvds_writel(lvds, LVDS_PxPLLSDCR1(phy), mdiv);
>> +	lvds_writel(lvds, LVDS_PxPLLTESTCR(phy), TEST_DIV << 16);
>> +
>> +	/* Disable TWG and SD: for now, PLL just need to be in integer mode */
>> +	lvds_clear(lvds, LVDS_PxPLLCR1(phy), EN_TWG | EN_SD);
>> +
>> +	/* Power up bias and PLL dividers */
>> +	lvds_set(lvds, LVDS_PxDCR(phy), POWER_OK);
>> +
>> +	lvds_set(lvds, LVDS_PxCMCR1(phy), CM_EN_DL);
>> +	lvds_set(lvds, LVDS_PxCMCR2(phy), CM_EN_DL4);
>> +
>> +	lvds_set(lvds, LVDS_PxPLLCPCR(phy), 0x1);
> Add a define to replace 0x1, for example #defince PLL_CHARGE_PUMP_CONTROL_DEFAULT 0x1 ?
Ack, I'll change for a define.
>
>> +	lvds_set(lvds, LVDS_PxBCR3(phy), VM_EN_DL);
>> +	lvds_set(lvds, LVDS_PxBCR1(phy), EN_BIAS_DL);
>> +	lvds_set(lvds, LVDS_PxCFGCR(phy), EN_DIG_DL);
>> +
>> +	/* lvds_pll_enable */
>> +	/* PLL lock timing control for the monitor unmask after startup (pll_en) */
>> +	/* Adjust the value so that the masking window is opened at start-up */
>> +	/* MST_MON_PLL_LOCK_UNMASK_TUNE */
>> +	lvds_writel(lvds, LVDS_PxMPLCR(phy), (0x200 - 0x160) << 16);
> What are 0x200 and 0x160 ? add a define with a meaningful name for each of these value.
Those are the max and min time for the window configuration to be open.
I'll define the sum of it directly.
>
>> +
>> +	lvds_writel(lvds, LVDS_PxBCR2(phy), BIAS_EN);
>> +
>> +	lvds_set(lvds, LVDS_PxGCR(phy),
>> +		 PHY_GCR_DP_CLK_OUT | PHY_GCR_LS_CLK_OUT | PHY_GCR_BIT_CLK_OUT);
>> +
>> +	/* TODO hardcoded values for now */
>> +	lvds_set(lvds, LVDS_PxPLLTESTCR(phy), BIT(8) /* PLL_TEST_DIV_EN */);
> Why hardcoded value for now ? simply add a define PLL_TEST_DIV_EN BIT(8) ?
Ack
>
>> +	lvds_set(lvds, LVDS_PxPLLCR1(phy), BIT(8) /* PLL_DIVIDERS_ENABLE */);
> ditto
Ack
>
>> +
>> +	lvds_set(lvds, LVDS_PxSCR(phy), BIT(16) /* SER_DATA_OK */);
> ditto
Ack
>> +
>> +	/* Enable the LVDS PLL & wait for its lock */
>> +	lvds_set(lvds, LVDS_PxPLLCR1(phy), PLL_EN);
>> +	ret = readl_poll_sleep_timeout(lvds->base + LVDS_PxPLLSR(phy),
>> +				       val, val & PLL_LOCK, SLEEP_US, TIMEOUT_US);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Select MST PHY clock as pixel clock for the LDITX instead of FREF */
>> +	/* WCLKCR_SLV_CLKPIX_SEL is for dual link */
>> +	lvds_writel(lvds, LVDS_WCLKCR, WCLKCR_SLV_CLKPIX_SEL);
>> +
>> +	lvds_set(lvds, LVDS_PxPLLTESTCR(phy), BIT(0));
> add #define PLL_TEST_CLK_EN BIT(0) instead ?
>
> lvds_set(lvds, LVDS_PxPLLTESTCR(phy), PLL_TEST_CLK_EN);
Ack
>
>
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_lvds_enable(struct udevice *dev,
>> +			     const struct display_timing *timings)
>> +{
>> +	struct stm32_lvds *lvds = dev_get_priv(dev);
>> +	u32 lvds_cdl1cr = 0;
>> +	u32 lvds_cdl2cr = 0;
>> +	u32 lvds_dmlcr = 0;
>> +	u32 lvds_dmmcr = 0;
>> +	u32 lvds_cr = 0;
>> +	int i;
>> +
>> +	lvds_clear(lvds, LVDS_CDL1CR, CDLCRx_DISTR0 | CDLCRx_DISTR1 | CDLCRx_DISTR2
>> +					| CDLCRx_DISTR3 | CDLCRx_DISTR4);
>> +	lvds_clear(lvds, LVDS_CDL2CR, CDLCRx_DISTR0 | CDLCRx_DISTR1 | CDLCRx_DISTR2
>> +					| CDLCRx_DISTR3 | CDLCRx_DISTR4);
>> +
>> +	/* Set channel distribution */
>> +	lvds_cr &= ~CR_LKMOD;
>> +	lvds_cdl1cr = CDL1CR_DEFAULT;
>> +
>> +	if (lvds->dual_link) {
>> +		lvds_cr |= CR_LKMOD;
>> +		lvds_cdl2cr = CDL2CR_DEFAULT;
>> +	}
>> +
>> +	/* Set signal polarity */
>> +	if (timings->flags & DISPLAY_FLAGS_DE_LOW)
>> +		lvds_cr |= CR_DEPOL;
>> +
>> +	if (timings->flags & DISPLAY_FLAGS_HSYNC_LOW)
>> +		lvds_cr |= CR_HSPOL;
>> +
>> +	if (timings->flags & DISPLAY_FLAGS_VSYNC_LOW)
>> +		lvds_cr |= CR_VSPOL;
>> +
>> +	/* Set link phase */
>> +	switch (lvds->dual_link) {
>> +	case LVDS_DUAL_LINK_EVEN_ODD_PIXELS: /* LKPHA = 0 */
>> +		lvds_cr &= ~CR_LKPHA;
>> +		break;
>> +	case LVDS_DUAL_LINK_ODD_EVEN_PIXELS: /* LKPHA = 1 */
>> +		lvds_cr |= CR_LKPHA;
>> +		break;
>> +	default:
>> +		dev_dbg(dev, "No phase precised, setting default\n");
>> +		lvds_cr &= ~CR_LKPHA;
>> +		break;
>> +	}
>> +
>> +	/* Set Data Mapping */
>> +	switch (lvds->bus_format) {
>> +	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: /* VESA-RGB888 */
>> +		for (i = 0; i < 5; i++) {
>> +			lvds_dmlcr = ((lvds_bitmap_vesa_rgb888[i][0])
>> +				      + (lvds_bitmap_vesa_rgb888[i][1] << 5)
>> +				      + (lvds_bitmap_vesa_rgb888[i][2] << 10)
>> +				      + (lvds_bitmap_vesa_rgb888[i][3] << 15));
>> +			lvds_dmmcr = ((lvds_bitmap_vesa_rgb888[i][4])
>> +				      + (lvds_bitmap_vesa_rgb888[i][5] << 5)
>> +				      + (lvds_bitmap_vesa_rgb888[i][6] << 10));
>> +
>> +			/* Write registers at the end of computations */
>> +			lvds_writel(lvds, LVDS_DMLCR(i), lvds_dmlcr);
>> +			lvds_writel(lvds, LVDS_DMMCR(i), lvds_dmmcr);
>> +		}
>> +		break;
>> +	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: /* JEIDA-RGB888 */
>> +		for (i = 0; i < 5; i++) {
>> +			lvds_dmlcr = ((lvds_bitmap_jeida_rgb888[i][0])
>> +				      + (lvds_bitmap_jeida_rgb888[i][1] << 5)
>> +				      + (lvds_bitmap_jeida_rgb888[i][2] << 10)
>> +				      + (lvds_bitmap_jeida_rgb888[i][3] << 15));
>> +			lvds_dmmcr = ((lvds_bitmap_jeida_rgb888[i][4])
>> +				      + (lvds_bitmap_jeida_rgb888[i][5] << 5)
>> +				      + (lvds_bitmap_jeida_rgb888[i][6] << 10));
>> +
>> +			/* Write registers at the end of computations */
>> +			lvds_writel(lvds, LVDS_DMLCR(i), lvds_dmlcr);
>> +			lvds_writel(lvds, LVDS_DMMCR(i), lvds_dmmcr);
>> +		}
>> +		break;
>> +	default:
>> +		dev_dbg(dev, "Unsupported LVDS bus format 0x%04x\n", lvds->bus_format);
>> +	}
>> +
>> +	/* Turn the output on */
>> +	lvds_cr |= CR_LVDSEN;
>> +
>> +	/* Commit config to registers */
>> +	lvds_set(lvds, LVDS_CR, lvds_cr);
>> +	lvds_writel(lvds, LVDS_CDL1CR, lvds_cdl1cr);
>> +	lvds_writel(lvds, LVDS_CDL2CR, lvds_cdl2cr);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_lvds_attach(struct udevice *dev)
>> +{
>> +	struct stm32_lvds *lvds = dev_get_priv(dev);
>> +	struct display_timing timings;
>> +	int ret;
>> +
>> +	ret = panel_get_display_timing(lvds->panel, &timings);
>> +	if (ret) {
>> +		ret = ofnode_decode_display_timing(dev_ofnode(lvds->panel),
>> +						   0, &timings);
>> +		if (ret) {
>> +			dev_err(dev, "decode display timing error %d\n", ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = stm32_lvds_enable(dev, &timings);
>> +
>> +	return ret;
>> +}
>> +
>> +static int stm32_lvds_set_backlight(struct udevice *dev, int percent)
>> +{
>> +	struct stm32_lvds *lvds = dev_get_priv(dev);
>> +	int ret;
>> +
>> +	ret = panel_enable_backlight(lvds->panel);
>> +	if (ret) {
>> +		dev_err(dev, "panel %s enable backlight error %d\n",
>> +			lvds->panel->name, ret);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int lvds_handle_pixel_order(struct stm32_lvds *lvds)
>> +{
>> +	ofnode parent, panel_port0, panel_port1;
>> +	bool even_pixels, odd_pixels;
>> +	int port0, port1;
>> +
>> +	/*
>> +	 * In case we are operating in single link,
>> +	 * there is only one port linked to the LVDS.
>> +	 * Check whether we are in this case and exit if yes.
>> +	 */
>> +	parent = ofnode_find_subnode(dev_ofnode(lvds->panel), "ports");
>> +	if (!ofnode_valid(parent))
>> +		return 0;
>> +
>> +	panel_port0 = ofnode_first_subnode(parent);
>> +	if (!ofnode_valid(panel_port0))
>> +		return -EPIPE;
>> +
>> +	even_pixels = ofnode_read_bool(panel_port0, "dual-lvds-even-pixels");
>> +	odd_pixels = ofnode_read_bool(panel_port0, "dual-lvds-odd-pixels");
>> +	if (even_pixels && odd_pixels)
>> +		return -EINVAL;
>> +
>> +	port0 = even_pixels ? LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
>> +		LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
>> +
>> +	panel_port1 = ofnode_next_subnode(panel_port0);
>> +	if (!ofnode_valid(panel_port1))
>> +		return -EPIPE;
>> +
>> +	even_pixels = ofnode_read_bool(panel_port1, "dual-lvds-even-pixels");
>> +	odd_pixels = ofnode_read_bool(panel_port1, "dual-lvds-odd-pixels");
>> +	if (even_pixels && odd_pixels)
>> +		return -EINVAL;
>> +
>> +	port1 = even_pixels ? LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
>> +		LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
>> +
>> +	/*
>> +	 * A valid dual-LVDS bus is found when one port is marked with
>> +	 * "dual-lvds-even-pixels", and the other port is marked with
>> +	 * "dual-lvds-odd-pixels", bail out if the markers are not right.
>> +	 */
>> +	if (port0 + port1 != LVDS_DUAL_LINK_EVEN_ODD_PIXELS + LVDS_DUAL_LINK_ODD_EVEN_PIXELS)
>> +		return -EINVAL;
>> +
>> +	return port0;
>> +}
>> +
>> +static int stm32_lvds_probe(struct udevice *dev)
>> +{
>> +	struct stm32_lvds *priv = dev_get_priv(dev);
>> +	struct display_timing timings;
>> +	struct reset_ctl rst;
>> +	struct clk pclk, refclk;
>> +	const char *data_mapping;
>> +	int ret;
>> +
>> +	priv->base = dev_read_addr_ptr(dev);
>> +	if ((fdt_addr_t)priv->base == FDT_ADDR_T_NONE) {
>> +		dev_err(dev, "Unable to read LVDS base address\n");
>> +		return -EINVAL;
>> +	}
>> +
> Device tree resources reading must be located in an of_to_plat() function, not in probe().
> See for example, see ./drivers/spi/stm32_spi.c
>
> For reference : https://docs.u-boot.org/en/stable/develop/driver-model/design.html#reading-ofdata
Ack.
>
>
>
>> +	ret = clk_get_by_name(dev, "pclk", &pclk);
>> +	if (ret) {
>> +		dev_err(dev, "Unable to get peripheral clock: %d\n", ret);
>> +		return ret;
> ditto
Ack
>> +	}
>> +
>> +	ret = clk_enable(&pclk);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable peripheral clock: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_get_by_name(dev, "ref", &refclk);
>> +	if (ret) {
>> +		dev_err(dev, "Unable to get reference clock: %d\n", ret);
>> +		goto err_clk;
>> +	}
> ditto
Ack
>
>> +
>> +	ret = clk_enable(&refclk);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable reference clock: %d\n", ret);
>> +		goto err_clk;
>> +	}
>> +
>> +	priv->refclk = (unsigned int)clk_get_rate(&refclk);
>> +
>> +	ret = reset_get_by_index(dev, 0, &rst);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to get LVDS reset: %d\n", ret);
>> +		goto err_rst;
>> +	}
>> +
> ditto
Ack
>
>> +	reset_deassert(&rst);
>> +
>> +	ret = uclass_get_device_by_driver(UCLASS_PANEL,
>> +					  DM_DRIVER_GET(simple_panel), &priv->panel);
>> +	if (ret) {
>> +		dev_err(dev, "panel device error %d\n", ret);
>> +		goto err_rst;
>> +	}
> ditto
Ack
>> +
>> +	ret = panel_get_display_timing(priv->panel, &timings);
>> +	if (ret) {
>> +		ret = ofnode_decode_display_timing(dev_ofnode(priv->panel),
>> +						   0, &timings);
>> +		if (ret) {
>> +			dev_err(dev, "decode display timing error %d\n", ret);
>> +			goto err_rst;
>> +		}
>> +	}
>> +
> ditto 
Ack
>
>> +	data_mapping = ofnode_read_string(dev_ofnode(priv->panel), "data-mapping");
>> +	if (!strcmp(data_mapping, "vesa-24"))
>> +		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
>> +	else if (!strcmp(data_mapping, "jeida-24"))
>> +		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
>> +	else
>> +		priv->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
> ditto
Ack.


Thanks for the review.

Best regards,
Raphaël
>
>> +
>> +	/* Handle dual link config */
>> +	priv->dual_link = lvds_handle_pixel_order(priv);
>> +	if (priv->dual_link < 0)
>> +		goto err_rst;
>> +
>> +	if (priv->dual_link > 0) {
>> +		ret = stm32_lvds_pll_enable(priv, &timings, LVDS_PHY_SLAVE);
>> +		if (ret)
>> +			goto err_rst;
>> +	}
>> +
>> +	ret = stm32_lvds_pll_enable(priv, &timings, LVDS_PHY_MASTER);
>> +	if (ret)
>> +		goto err_rst;
>> +
>> +	return 0;
>> +
>> +err_rst:
>> +	clk_disable(&refclk);
>> +err_clk:
>> +	clk_disable(&pclk);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct video_bridge_ops stm32_lvds_ops = {
>> +	.attach = stm32_lvds_attach,
>> +	.set_backlight = stm32_lvds_set_backlight,
>> +};
>> +
>> +static const struct udevice_id stm32_lvds_ids[] = {
>> +	{.compatible = "st,stm32mp25-lvds"},
>> +	{}
>> +};
>> +
>> +U_BOOT_DRIVER(stm32_lvds) = {
>> +	.name		= "stm32-display-lvds",
>> +	.id		= UCLASS_VIDEO_BRIDGE,
>> +	.of_match	= stm32_lvds_ids,
>> +	.ops		= &stm32_lvds_ops,
>> +	.probe		= stm32_lvds_probe,
>> +	.priv_auto	= sizeof(struct stm32_lvds),
>> +};
>>

Re: [PATCH 7/8] ARM: dts: stm32: remove panel property in stm32mp257f-ev1-u-boot

[Raphael Gallais-Pou]

On 8/25/25 15:54, Patrice CHOTARD wrote:
>
> On 8/20/25 18:17, Raphael Gallais-Pou wrote:
>> SCMI regulators are not yet implemented in OP-TEE.  This leads to an
>> invalid argument error when retrieving the 'power-supply' property.
>> Ultimately this chains into the panel not probing and makes the whole
>> display pipe fail.
>>
>> Remove the property in the upstream device-tree to allow display pipe to
>> boot.
>>
>> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
>> ---
>>  arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
>> index 9a566e18d3fd9b0cf4e21605f2cb060b49fadcb6..7a4b623664a1e167ccc0f4c73a979a083337d7a2 100644
>> --- a/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
>> +++ b/arch/arm/dts/stm32mp257f-ev1-u-boot.dtsi
>> @@ -12,6 +12,10 @@
>>  	};
>>  };
>>  
>> +&panel_lvds {
>> +	/delete-property/ power-supply;
>> +};
>> +
>>  &usart2 {
>>  	bootph-all;
>>  };
>>
> This patch is a temporary patch right ?
> If yes, i am not in favor to applied this patch, i prefer to see that there is something missing.
>
> Thanks

Hi Patrice,

Yes it is temporary.  I will remove this patch from the upcoming series.
I understand your intention, just mind that the display pipe won't work until
STM32 OP-TEE gets SCMI regulators implemented.

Best regards,
Raphaël