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-...@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-...@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),
>> +};
>>
