[PATCH 1/3] video: stm: make multi-image compatible

[PATCH 1/3] video: stm: make multi-image compatible

[Ahmad Fatoum]

Unlike newer platforms, it's not possible to mix i.MX23 and i.MX28 SoC
support in the same build. The stmfb driver is used on newer SoCs
however like the i.MX6SX and i.MX8MQ/M/N, so remove the #ifdeffery in
the driver.

While at it, we also drop unused macros as well as the unused dotclk_delay.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 .../arm/boards/chumby_falconwing/falconwing.c |   1 +
 arch/arm/boards/karo-tx28/tx28-stk5.c         |   1 +
 drivers/video/Kconfig                         |   2 +-
 drivers/video/stm.c                           | 115 ++++++++++--------
 include/mach/mxs/fb.h                         |  24 ++++
 5 files changed, 93 insertions(+), 50 deletions(-)

diff --git a/arch/arm/boards/chumby_falconwing/falconwing.c b/arch/arm/boards/chumby_falconwing/falconwing.c
index 922159045540..e884c9e108bf 100644
--- a/arch/arm/boards/chumby_falconwing/falconwing.c
+++ b/arch/arm/boards/chumby_falconwing/falconwing.c
@@ -76,6 +76,7 @@ static struct imx_fb_platformdata fb_mode = {
 	.enable = chumby_fb_enable,
 	.fixed_screen = (void *)(0x40000000 + SZ_64M - MAX_FB_SIZE),
 	.fixed_screen_size = MAX_FB_SIZE,
+	.devdata = MXSFB_DEVDATA(MXSFB_V3),
 };
 
 static const uint32_t pad_setup[] = {
diff --git a/arch/arm/boards/karo-tx28/tx28-stk5.c b/arch/arm/boards/karo-tx28/tx28-stk5.c
index 737691d68b61..09b4e2755800 100644
--- a/arch/arm/boards/karo-tx28/tx28-stk5.c
+++ b/arch/arm/boards/karo-tx28/tx28-stk5.c
@@ -192,6 +192,7 @@ static struct imx_fb_platformdata tx28_fb_pdata = {
 	.fixed_screen = (void *)(0x40000000 + SZ_128M - MAX_FB_SIZE),
 	.fixed_screen_size = MAX_FB_SIZE,
 	.enable = tx28_fb_enable,
+	.devdata = MXSFB_DEVDATA(MXSFB_V4),
 };
 
 static const uint32_t tx28_starterkit_pad_setup[] = {
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4605634bb581..192e05bd59d8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -54,7 +54,7 @@ config DRIVER_VIDEO_IMX_IPU_OVERLAY
 
 config DRIVER_VIDEO_STM
 	bool "i.MX23/28 framebuffer driver"
-	depends on ARCH_MXS
+	depends on ARCH_MXS || COMPILE_TEST
 	help
 	  Say 'Y' here to enable framebuffer and splash screen support for
 	  i.MX23 and i.MX28 based systems.
diff --git a/drivers/video/stm.c b/drivers/video/stm.c
index 86510b939790..c5dadb823251 100644
--- a/drivers/video/stm.c
+++ b/drivers/video/stm.c
@@ -8,6 +8,9 @@
  * Copyright 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
  * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved.
  */
+
+#define pr_fmt(fmt) "stmfb: " fmt
+
 #include <common.h>
 #include <init.h>
 #include <driver.h>
@@ -47,24 +50,16 @@
 # define GET_BYTE_PACKAGING(x) (((x) >> 16) & 0xf)
 # define CTRL1_RESET (1 << 0)
 
-#ifdef CONFIG_ARCH_IMX28
-# define HW_LCDIF_CTRL2 0x20
-# define HW_LCDIF_TRANSFER_COUNT 0x30
-#endif
-#ifdef CONFIG_ARCH_IMX23
-# define HW_LCDIF_TRANSFER_COUNT 0x20
-#endif
+#define LCDC_V3_TRANSFER_COUNT		0x20
+#define LCDC_V4_TRANSFER_COUNT		0x30
+
 # define SET_VCOUNT(x) (((x) & 0xffff) << 16)
 # define SET_HCOUNT(x) ((x) & 0xffff)
 
-#ifdef CONFIG_ARCH_IMX28
-# define HW_LCDIF_CUR_BUF 0x40
-# define HW_LCDIF_NEXT_BUF 0x50
-#endif
-#ifdef CONFIG_ARCH_IMX23
-# define HW_LCDIF_CUR_BUF 0x30
-# define HW_LCDIF_NEXT_BUF 0x40
-#endif
+#define LCDC_V3_CUR_BUF			0x30
+#define LCDC_V3_NEXT_BUF		0x40
+#define LCDC_V4_CUR_BUF			0x40
+#define LCDC_V4_NEXT_BUF		0x50
 
 #define HW_LCDIF_TIMING 0x60
 # define SET_CMD_HOLD(x) (((x) & 0xff) << 24)
@@ -87,12 +82,8 @@
 #define HW_LCDIF_VDCTRL1 0x80
 
 #define HW_LCDIF_VDCTRL2 0x90
-#ifdef CONFIG_ARCH_IMX28
-# define SET_HSYNC_PULSE_WIDTH(x) (((x) & 0x3fff) << 18)
-#endif
-#ifdef CONFIG_ARCH_IMX23
-# define SET_HSYNC_PULSE_WIDTH(x) (((x) & 0xff) << 24)
-#endif
+# define SET_HSYNC_PULSE_WIDTH(devdata, x) \
+	(((x) & (devdata)->hs_wdth_mask) << (devdata)->hs_wdth_shift)
 # define SET_HSYNC_PERIOD(x) ((x) & 0x3ffff)
 
 #define HW_LCDIF_VDCTRL3 0xa0
@@ -102,9 +93,6 @@
 # define SET_VERT_WAIT_CNT(x) ((x) & 0xffff)
 
 #define HW_LCDIF_VDCTRL4 0xb0
-#ifdef CONFIG_ARCH_IMX28
-# define SET_DOTCLK_DLY(x) (((x) & 0x7) << 29)
-#endif
 # define VDCTRL4_SYNC_SIGNALS_ON (1 << 18)
 # define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3ffff)
 
@@ -114,19 +102,9 @@
 #define HW_LCDIF_DVICTRL3 0xf0
 #define HW_LCDIF_DVICTRL4 0x100
 
-#ifdef CONFIG_ARCH_IMX28
-# define HW_LCDIF_DATA 0x180
-#endif
-#ifdef CONFIG_ARCH_IMX23
-# define HW_LCDIF_DATA 0x1b0
-#endif
+#define LCDC_V3_DEBUG0 0x1f0
+#define LCDC_V4_DEBUG0 0x1d0
 
-#ifdef CONFIG_ARCH_IMX28
-# define HW_LCDIF_DEBUG0 0x1d0
-#endif
-#ifdef CONFIG_ARCH_IMX23
-# define HW_LCDIF_DEBUG0 0x1f0
-#endif
 # define DEBUG_HSYNC (1 < 26)
 # define DEBUG_VSYNC (1 < 25)
 
@@ -146,7 +124,7 @@ struct imxfb_info {
 	unsigned flags;
 	unsigned ld_intf_width;
 	void (*enable)(int enable);
-	unsigned dotclk_delay;
+	const struct mxsfb_devdata *devdata;
 };
 
 /* the RGB565 true colour mode */
@@ -216,6 +194,7 @@ static inline unsigned calc_line_length(unsigned ppl, unsigned bpp)
 static void stmfb_enable_controller(struct fb_info *fb_info)
 {
 	struct imxfb_info *fbi = fb_info->priv;
+	const struct mxsfb_devdata *devdata = fbi->devdata;
 	uint32_t reg, last_reg;
 	unsigned loop, edges;
 
@@ -244,9 +223,9 @@ static void stmfb_enable_controller(struct fb_info *fb_info)
 
 	while (edges != 0) {
 		loop = 800;
-		last_reg = readl(fbi->base + HW_LCDIF_DEBUG0) & DEBUG_VSYNC;
+		last_reg = readl(fbi->base + devdata->debug0) & DEBUG_VSYNC;
 		do {
-			reg = readl(fbi->base + HW_LCDIF_DEBUG0) & DEBUG_VSYNC;
+			reg = readl(fbi->base + devdata->debug0) & DEBUG_VSYNC;
 			if (reg != last_reg)
 				break;
 			last_reg = reg;
@@ -307,6 +286,7 @@ static void stmfb_disable_controller(struct fb_info *fb_info)
 static int stmfb_activate_var(struct fb_info *fb_info)
 {
 	struct imxfb_info *fbi = fb_info->priv;
+	const struct mxsfb_devdata *devdata = fbi->devdata;
 	struct fb_videomode *mode = fb_info->mode;
 	uint32_t reg;
 	unsigned size;
@@ -430,8 +410,8 @@ static int stmfb_activate_var(struct fb_info *fb_info)
 	writel(reg, fbi->base + HW_LCDIF_CTRL);
 	pr_debug("Setting up CTRL to %08X\n", reg);
 
-	writel(SET_VCOUNT(mode->yres) |
-		SET_HCOUNT(mode->xres), fbi->base + HW_LCDIF_TRANSFER_COUNT);
+	writel(SET_VCOUNT(mode->yres) | SET_HCOUNT(mode->xres),
+	       fbi->base + devdata->transfer_count);
 
 	reg = VDCTRL0_ENABLE_PRESENT |	/* always in DOTCLOCK mode */
 		VDCTRL0_VSYNC_PERIOD_UNIT |
@@ -457,7 +437,7 @@ static int stmfb_activate_var(struct fb_info *fb_info)
 		fbi->base + HW_LCDIF_VDCTRL1);
 
 	/* line length in units of clocks or pixels */
-	writel(SET_HSYNC_PULSE_WIDTH(mode->hsync_len) |
+	writel(SET_HSYNC_PULSE_WIDTH(devdata, mode->hsync_len) |
 		SET_HSYNC_PERIOD(mode->left_margin + mode->hsync_len +
 			mode->right_margin + mode->xres),
 		fbi->base + HW_LCDIF_VDCTRL2);
@@ -466,15 +446,11 @@ static int stmfb_activate_var(struct fb_info *fb_info)
 		SET_VERT_WAIT_CNT(mode->upper_margin + mode->vsync_len),
 		fbi->base + HW_LCDIF_VDCTRL3);
 
-	writel(
-#ifdef CONFIG_ARCH_IMX28
-		SET_DOTCLK_DLY(fbi->dotclk_delay) |
-#endif
-		SET_DOTCLK_H_VALID_DATA_CNT(mode->xres),
+	writel(SET_DOTCLK_H_VALID_DATA_CNT(mode->xres),
 		fbi->base + HW_LCDIF_VDCTRL4);
 
-	writel((uint32_t)fb_info->screen_base, fbi->base + HW_LCDIF_CUR_BUF);
-	writel((uint32_t)fb_info->screen_base, fbi->base + HW_LCDIF_NEXT_BUF);
+	writel((uintptr_t)fb_info->screen_base, fbi->base + devdata->cur_buf);
+	writel((uintptr_t)fb_info->screen_base, fbi->base + devdata->next_buf);
 
 	return 0;
 }
@@ -519,6 +495,21 @@ static int stmfb_probe(struct device *hw_dev)
 
 	/* add runtime video info */
 	if (pdata) {
+		if (!pdata->devdata) {
+			const char *name;
+
+			if (IS_ENABLED(CONFIG_ARCH_IMX23)) {
+				pdata->devdata = &mxsfb_devdata[MXSFB_V3];
+				name = "MXSFB_V3";
+			} else {
+				pdata->devdata = &mxsfb_devdata[MXSFB_V4];
+				name = "MXSFB_V4";
+			}
+
+			pr_warn("imx_fb_platformdata::devdata unset. Assuming %s\n",
+				name);
+		}
+
 		fbi.info.modes.modes = pdata->mode_list;
 		fbi.info.modes.num_modes = pdata->mode_cnt;
 		fbi.info.mode = &fbi.info.modes.modes[0];
@@ -536,6 +527,10 @@ static int stmfb_probe(struct device *hw_dev)
 		if (!IS_ENABLED(CONFIG_OFDEVICE) || !hw_dev->of_node)
 			return -EINVAL;
 
+		fbi.devdata = device_get_match_data(hw_dev);
+		if (!fbi.devdata)
+			return -EINVAL;
+
 		display = of_parse_phandle(hw_dev->of_node, "display", 0);
 		if (!display) {
 			dev_err(hw_dev, "no display phandle\n");
@@ -573,11 +568,33 @@ static int stmfb_probe(struct device *hw_dev)
 	return 0;
 }
 
+const struct mxsfb_devdata mxsfb_devdata[] = {
+	[MXSFB_V3] = {
+		.transfer_count	= LCDC_V3_TRANSFER_COUNT,
+		.cur_buf	= LCDC_V3_CUR_BUF,
+		.next_buf	= LCDC_V3_NEXT_BUF,
+		.debug0		= LCDC_V3_DEBUG0,
+		.hs_wdth_mask	= 0xff,
+		.hs_wdth_shift	= 24,
+	},
+	[MXSFB_V4] = {
+		.transfer_count	= LCDC_V4_TRANSFER_COUNT,
+		.cur_buf	= LCDC_V4_CUR_BUF,
+		.next_buf	= LCDC_V4_NEXT_BUF,
+		.debug0		= LCDC_V4_DEBUG0,
+		.hs_wdth_mask	= 0x3fff,
+		.hs_wdth_shift	= 18,
+	},
+};
+EXPORT_SYMBOL(mxsfb_devdata);
+
 static __maybe_unused struct of_device_id stmfb_compatible[] = {
 	{
 		.compatible = "fsl,imx23-lcdif",
+		.data = &mxsfb_devdata[MXSFB_V3],
 	}, {
 		.compatible = "fsl,imx28-lcdif",
+		.data = &mxsfb_devdata[MXSFB_V4],
 	}, {
 		/* sentinel */
 	}
diff --git a/include/mach/mxs/fb.h b/include/mach/mxs/fb.h
index 8fcfe8652067..da2ed2f2ebee 100644
--- a/include/mach/mxs/fb.h
+++ b/include/mach/mxs/fb.h
@@ -18,6 +18,28 @@
 
 #define USE_LCD_RESET		1
 
+enum mxsfb_devtype {
+	MXSFB_V3,
+	MXSFB_V4,
+};
+
+struct mxsfb_devdata {
+	unsigned int	transfer_count;
+	unsigned int	cur_buf;
+	unsigned int	next_buf;
+	unsigned int	debug0;
+	unsigned int	hs_wdth_mask;
+	unsigned int	hs_wdth_shift;
+};
+
+extern const struct mxsfb_devdata mxsfb_devdata[];
+
+#ifdef CONFIG_DRIVER_VIDEO_STM
+#define MXSFB_DEVDATA(devtype) (&mxsfb_devdata[devtype])
+#else
+#define MXSFB_DEVDATA(devtype) 0
+#endif
+
 struct imx_fb_platformdata {
 	struct fb_videomode *mode_list;
 	unsigned mode_cnt;
@@ -31,6 +53,8 @@ struct imx_fb_platformdata {
 
 	unsigned flags;
 	void (*enable)(int enable); /**< hook to enable backlight */
+
+	const struct mxsfb_devdata *devdata;
 };
 
 #endif /* __MACH_FB_H */
-- 
2.47.3

[PATCH 2/3] video: stm: allocate uncached framebuffer memory as write-combine

[Ahmad Fatoum]

For i.MX23 and i.MX28, this makes no difference as the MMU treats
write-combine and uncached the same on ARMv6 and earlier, but on
ARMv7 and above, use of the write combine buffer allows us to spend less
time pushing pixels.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 drivers/video/stm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/stm.c b/drivers/video/stm.c
index c5dadb823251..da006b7c240f 100644
--- a/drivers/video/stm.c
+++ b/drivers/video/stm.c
@@ -305,8 +305,8 @@ static int stmfb_activate_var(struct fb_info *fb_info)
 		remap_range(fbi->fixed_screen,
 				fbi->fixed_screen_size, MAP_UNCACHED);
 	} else {
-		fb_info->screen_base = dma_alloc_coherent(DMA_DEVICE_BROKEN,
-							  size, DMA_ADDRESS_BROKEN);
+		fb_info->screen_base = dma_alloc_writecombine(DMA_DEVICE_BROKEN,
+							      size, DMA_ADDRESS_BROKEN);
 		if (!fb_info->screen_base)
 			return -ENOMEM;
 		fbi->memory_size = size;
-- 
2.47.3

[PATCH 3/3] video: stm: add explicit support for i.MX6SX

[Ahmad Fatoum]

To anticipate a possible upstream change that removes "fsl,imx28-lcdif"
from the compatible list for the i.MX6SX LCDIF node, add an explicit
match for "fsl,imx6sx-lcdif".

While at it, adapt the driver for better compatibility with the SoloX:

  - Enable recovery on underflow, which has been observed under Linux to
    cause pixel shifts
  - Skip the VSYNC loop. The register doesn't exist on the i.MX6SX and
    the loop would just count down 3200 reads of whatever happens
    to be at that MMIO address

These changes have only been compile tested.

Cc: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 drivers/video/stm.c   | 75 ++++++++++++++++++++++++++++++++++---------
 include/mach/mxs/fb.h |  5 +++
 2 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/drivers/video/stm.c b/drivers/video/stm.c
index da006b7c240f..49f7c356f274 100644
--- a/drivers/video/stm.c
+++ b/drivers/video/stm.c
@@ -45,6 +45,7 @@
 # define CTRL_RUN (1 << 0)
 
 #define HW_LCDIF_CTRL1 0x10
+#define CTRL1_RECOVER_ON_UNDERFLOW	(1 << 24)
 # define CTRL1_FIFO_CLEAR (1 << 21)
 # define SET_BYTE_PACKAGING(x) (((x) & 0xf) << 16)
 # define GET_BYTE_PACKAGING(x) (((x) >> 16) & 0xf)
@@ -215,23 +216,57 @@ static void stmfb_enable_controller(struct fb_info *fb_info)
 	writel(reg, fbi->base + HW_LCDIF_VDCTRL4);
 
 	/*
-	 * Give the attached LC display or monitor a chance to sync into
-	 * our signals.
-	 * Wait for at least 2 VSYNCs = four VSYNC edges
+	 * Enable recovery on underflow.
+	 *
+	 * There is some sort of corner case behavior of the controller,
+	 * which could rarely be triggered at least on i.MX6SX connected
+	 * to 800x480 DPI panel and i.MX8MM connected to DPI->DSI->LVDS
+	 * bridged 1920x1080 panel (and likely on other setups too), where
+	 * the image on the panel shifts to the right and wraps around.
+	 * This happens either when the controller is enabled on boot or
+	 * even later during run time. The condition does not correct
+	 * itself automatically, i.e. the display image remains shifted.
+	 *
+	 * It seems this problem is known and is due to sporadic underflows
+	 * of the LCDIF FIFO. While the LCDIF IP does have underflow/overflow
+	 * IRQs, neither of the IRQs trigger and neither IRQ status bit is
+	 * asserted when this condition occurs.
+	 *
+	 * All known revisions of the LCDIF IP have CTRL1 RECOVER_ON_UNDERFLOW
+	 * bit, which is described in the reference manual since i.MX23 as
+	 * "
+	 *   Set this bit to enable the LCDIF block to recover in the next
+	 *   field/frame if there was an underflow in the current field/frame.
+	 * "
+	 * Enable this bit to mitigate the sporadic underflows.
 	 */
-	edges = 4;
+	reg = readl(fbi->base + HW_LCDIF_CTRL1);
+	reg |= CTRL1_RECOVER_ON_UNDERFLOW;
+	writel(reg, fbi->base + HW_LCDIF_CTRL1);
 
-	while (edges != 0) {
-		loop = 800;
-		last_reg = readl(fbi->base + devdata->debug0) & DEBUG_VSYNC;
-		do {
-			reg = readl(fbi->base + devdata->debug0) & DEBUG_VSYNC;
-			if (reg != last_reg)
-				break;
-			last_reg = reg;
-			loop--;
-		} while (loop != 0);
-		edges--;
+	if (devdata->debug0) {
+		/*
+		 * Give the attached LC display or monitor a chance to sync into
+		 * our signals.
+		 * Wait for at least 2 VSYNCs = four VSYNC edges
+		 *
+		 * TODO: i.MX6SX doesn't have debug0. The Linux driver seems to
+		 * do without this at all..
+		 */
+		edges = 4;
+
+		while (edges != 0) {
+			loop = 800;
+			last_reg = readl(fbi->base + devdata->debug0) & DEBUG_VSYNC;
+			do {
+				reg = readl(fbi->base + devdata->debug0) & DEBUG_VSYNC;
+				if (reg != last_reg)
+					break;
+				last_reg = reg;
+				loop--;
+			} while (loop != 0);
+			edges--;
+		}
 	}
 
 	/* stop FIFO reset */
@@ -585,6 +620,13 @@ const struct mxsfb_devdata mxsfb_devdata[] = {
 		.hs_wdth_mask	= 0x3fff,
 		.hs_wdth_shift	= 18,
 	},
+	[MXSFB_V6] = {
+		.transfer_count	= LCDC_V4_TRANSFER_COUNT,
+		.cur_buf	= LCDC_V4_CUR_BUF,
+		.next_buf	= LCDC_V4_NEXT_BUF,
+		.hs_wdth_mask	= 0x3fff,
+		.hs_wdth_shift	= 18,
+	},
 };
 EXPORT_SYMBOL(mxsfb_devdata);
 
@@ -595,6 +637,9 @@ static __maybe_unused struct of_device_id stmfb_compatible[] = {
 	}, {
 		.compatible = "fsl,imx28-lcdif",
 		.data = &mxsfb_devdata[MXSFB_V4],
+	}, {
+		.compatible = "fsl,imx6sx-lcdif",
+		.data = &mxsfb_devdata[MXSFB_V6],
 	}, {
 		/* sentinel */
 	}
diff --git a/include/mach/mxs/fb.h b/include/mach/mxs/fb.h
index da2ed2f2ebee..b365816bf159 100644
--- a/include/mach/mxs/fb.h
+++ b/include/mach/mxs/fb.h
@@ -21,6 +21,11 @@
 enum mxsfb_devtype {
 	MXSFB_V3,
 	MXSFB_V4,
+	/*
+	 * Starting at i.MX6 the hardware version register is gone, use the
+	 * i.MX family number as the version.
+	 */
+	MXSFB_V6,
 };
 
 struct mxsfb_devdata {
-- 
2.47.3

Re: [PATCH 1/3] video: stm: make multi-image compatible

[Sascha Hauer]

On Fri, 07 Nov 2025 22:00:29 +0100, Ahmad Fatoum wrote:
> Unlike newer platforms, it's not possible to mix i.MX23 and i.MX28 SoC
> support in the same build. The stmfb driver is used on newer SoCs
> however like the i.MX6SX and i.MX8MQ/M/N, so remove the #ifdeffery in
> the driver.
> 
> While at it, we also drop unused macros as well as the unused dotclk_delay.
> 
> [...]

Applied, thanks!

[1/3] video: stm: make multi-image compatible
      https://git.pengutronix.de/cgit/barebox/commit/?id=c99b937f3a31 (link may not be stable)
[2/3] video: stm: allocate uncached framebuffer memory as write-combine
      https://git.pengutronix.de/cgit/barebox/commit/?id=cb4ab1e6ffc3 (link may not be stable)
[3/3] video: stm: add explicit support for i.MX6SX
      https://git.pengutronix.de/cgit/barebox/commit/?id=2ddbc8d9d15c (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>