* [PATCH 1/6] clk: imx8mp: add 700 MHz rate entry for VIDEO_PLL1
2026-06-02 4:09 [PATCH 0/7] video: enable boot splash on i.MX8MP with LVDS panel Johannes Schneider
@ 2026-06-02 4:09 ` Johannes Schneider
2026-06-02 7:04 ` Ahmad Fatoum
2026-06-02 4:09 ` [PATCH 2/6] pmdomain: imx8mp-blk-ctrl: add media blk-ctrl power domain support Johannes Schneider
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schneider @ 2026-06-02 4:09 UTC (permalink / raw)
To: barebox; +Cc: thomas.haemmerle
From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
The LVDS serializer requires link_freq = pixel_clock × 7. For a 25 MHz
pixel clock the link frequency is 175 MHz. VIDEO_PLL1 must be set to
700 MHz so the divider chain (VIDEO_PLL1 / 4 = 175 MHz) produces an
exact match; without it clk_set_rate() rounds down to 650 MHz, yielding
only 162.5 MHz and causing the LVDS transmitter to output ~743 pixels
per line instead of 800 — visible as a black strip on the right edge.
PLL parameters: Fin=24 MHz, P=3, S=2, K=0 → 24 × 350 / (3 × 4) = 700 MHz.
Assisted-by: Claude:claude-sonnet-4-6
Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
---
drivers/clk/imx/clk-pll14xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 6716a6f555..db7e744e87 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -61,6 +61,7 @@ static const struct imx_pll14xx_rate_table imx_pll1416x_tbl[] = {
};
static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
+ PLL_1443X_RATE(700000000U, 350, 3, 2, 0),
PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
base-commit: 81fbe2e8d0d445032498a0bfecf9fd270f00985a
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/6] clk: imx8mp: add 700 MHz rate entry for VIDEO_PLL1
2026-06-02 4:09 ` [PATCH 1/6] clk: imx8mp: add 700 MHz rate entry for VIDEO_PLL1 Johannes Schneider
@ 2026-06-02 7:04 ` Ahmad Fatoum
0 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2026-06-02 7:04 UTC (permalink / raw)
To: Johannes Schneider, barebox; +Cc: thomas.haemmerle
On 6/2/26 6:09 AM, Johannes Schneider wrote:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> The LVDS serializer requires link_freq = pixel_clock × 7. For a 25 MHz
> pixel clock the link frequency is 175 MHz. VIDEO_PLL1 must be set to
> 700 MHz so the divider chain (VIDEO_PLL1 / 4 = 175 MHz) produces an
> exact match; without it clk_set_rate() rounds down to 650 MHz, yielding
> only 162.5 MHz and causing the LVDS transmitter to output ~743 pixels
> per line instead of 800 — visible as a black strip on the right edge.
>
> PLL parameters: Fin=24 MHz, P=3, S=2, K=0 → 24 × 350 / (3 × 4) = 700 MHz.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/clk/imx/clk-pll14xx.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index 6716a6f555..db7e744e87 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -61,6 +61,7 @@ static const struct imx_pll14xx_rate_table imx_pll1416x_tbl[] = {
> };
>
> static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
> + PLL_1443X_RATE(700000000U, 350, 3, 2, 0),
> PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
> PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
>
> base-commit: 81fbe2e8d0d445032498a0bfecf9fd270f00985a
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/6] pmdomain: imx8mp-blk-ctrl: add media blk-ctrl power domain support
2026-06-02 4:09 [PATCH 0/7] video: enable boot splash on i.MX8MP with LVDS panel Johannes Schneider
2026-06-02 4:09 ` [PATCH 1/6] clk: imx8mp: add 700 MHz rate entry for VIDEO_PLL1 Johannes Schneider
@ 2026-06-02 4:09 ` Johannes Schneider
2026-06-02 7:34 ` Ahmad Fatoum
2026-06-02 4:09 ` [PATCH 3/6] video: backlight-pwm: make power-supply and enable-gpio optional Johannes Schneider
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schneider @ 2026-06-02 4:09 UTC (permalink / raw)
To: barebox; +Cc: thomas.haemmerle
From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
The i.MX8MP LCDIF2 controller lives inside the MEDIAMIX power domain,
which has its own blk-ctrl to gate clocks and control resets. Without
enabling the MEDIAMIX bus clock and de-asserting the block-level resets
before the first register access, any read or write to LCDIF2 registers
causes an AXI bus hang and barebox locks up on startup.
Extend the existing blk-ctrl driver (currently HSIO-only) to support the
media blk-ctrl (fsl,imx8mp-media-blk-ctrl) by:
- Separating the HSIO ADB handshake into the HSIO-specific power_on hook
so the generic path is clean for other blk-ctrls without such handshake.
- Adding null checks on bc->power_on/off to allow instances without custom
callbacks.
- Increasing DOMAIN_MAX_CLKS from 2 to 3 (LCDIF2 needs disp2, axi, apb).
- Adding media blk-ctrl probe that enables the MEDIAMIX bus clock and
de-asserts LCDIF2 clocks/resets, mirroring what the Linux kernel does
in imx8mp_media_power_notifier.
The media_blk_ctrl DTS node retains its syscon compatible so that
syscon_node_to_regmap() continues to work for the LDB bridge sub-device.
Assisted-by: Claude:claude-sonnet-4-6
Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
---
drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 117 +++++++++++++++++++++++--
1 file changed, 108 insertions(+), 9 deletions(-)
diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
index 3d302cbde7..87e0114c22 100644
--- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
+++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
@@ -17,6 +17,7 @@
#include <dt-bindings/power/imx8mp-power.h>
+/* HSIO blk-ctrl registers */
#define GPR_REG0 0x0
#define PCIE_CLOCK_MODULE_EN BIT(0)
#define USB_CLOCK_MODULE_EN BIT(1)
@@ -32,6 +33,11 @@
#define PLL_CKE BIT(17)
#define PLL_RST BIT(31)
+/* Media blk-ctrl registers */
+#define LCDIF_ARCACHE_CTRL 0x40
+#define LCDIF_1_RD_HURRY GENMASK(6, 4)
+#define LCDIF_0_RD_HURRY GENMASK(2, 0)
+
struct imx8mp_blk_ctrl_domain;
struct imx8mp_blk_ctrl {
@@ -51,7 +57,7 @@ struct imx8mp_blk_ctrl_domain_data {
const char *gpc_name;
};
-#define DOMAIN_MAX_CLKS 2
+#define DOMAIN_MAX_CLKS 3
struct imx8mp_blk_ctrl_domain {
struct generic_pm_domain genpd;
@@ -155,9 +161,14 @@ static int imx8mp_hsio_blk_ctrl_probe(struct imx8mp_blk_ctrl *bc)
return of_clk_add_hw_provider(dev_of_node(bc->dev), of_clk_hw_simple_get, hw);
}
+static int imx8mp_hsio_propagate_adb_handshake(struct imx8mp_blk_ctrl *bc);
+
static void imx8mp_hsio_blk_ctrl_power_on(struct imx8mp_blk_ctrl *bc,
struct imx8mp_blk_ctrl_domain *domain)
{
+ /* propagate ADB handshake once before any HSIO sub-domain is enabled */
+ imx8mp_hsio_propagate_adb_handshake(bc);
+
switch (domain->id) {
case IMX8MP_HSIOBLK_PD_USB:
regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
@@ -259,6 +270,66 @@ static const struct imx8mp_blk_ctrl_data imx8mp_hsio_blk_ctl_dev_data = {
.num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data),
};
+/* Media blk-ctrl */
+
+/*
+ * MEDIAMIX BLK_CTRL register offsets (from i.MX 8M Plus RM, section 13).
+ * Bit SET = reset de-asserted / clock enabled.
+ */
+#define BLK_SFT_RSTN 0x00
+#define BLK_CLK_EN 0x04
+
+/* BIT(8): bus/APB clock and reset for the MEDIAMIX interconnect */
+#define MEDIAMIX_BUS_CLK_RST BIT(8)
+
+/*
+ * LCDIF2 (mediablk-lcdif-2) bits in BLK_SFT_RSTN and BLK_CLK_EN:
+ * BIT(11): lcdif2-axi, BIT(12): lcdif2-apb, BIT(24): disp2 pixel clock
+ * (from Linux kernel imx8m-blk-ctrl.c IMX8MP_MEDIABLK_PD_LCDIF_2)
+ */
+#define LCDIF2_CLK_RST_MASK (BIT(11) | BIT(12) | BIT(24))
+
+static int imx8mp_media_blk_ctrl_probe(struct imx8mp_blk_ctrl *bc)
+{
+ /* Set panic read hurry level for LCDIF interfaces to 7 */
+ regmap_update_bits(bc->regmap, LCDIF_ARCACHE_CTRL,
+ FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
+ FIELD_PREP(LCDIF_0_RD_HURRY, 7),
+ FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
+ FIELD_PREP(LCDIF_0_RD_HURRY, 7));
+
+ /*
+ * Enable the MEDIAMIX bus clock and de-assert its reset so the
+ * internal AHB/APB fabric is up before sub-module access.
+ * (Mirrors Linux kernel imx8mp_media_power_notifier BIT(8) writes.)
+ */
+ regmap_set_bits(bc->regmap, BLK_CLK_EN, MEDIAMIX_BUS_CLK_RST);
+ regmap_set_bits(bc->regmap, BLK_SFT_RSTN, MEDIAMIX_BUS_CLK_RST);
+ udelay(5); /* wait for ADB handshake, as Linux kernel does */
+
+ /*
+ * Enable LCDIF2 clocks and de-assert its reset within MEDIAMIX.
+ * Without this, LCDIF2 registers are inaccessible (AXI bus hangs).
+ * (Mirrors Linux kernel imx8mp_blk_ctrl_power_on for LCDIF_2 domain.)
+ */
+ regmap_set_bits(bc->regmap, BLK_CLK_EN, LCDIF2_CLK_RST_MASK);
+ regmap_set_bits(bc->regmap, BLK_SFT_RSTN, LCDIF2_CLK_RST_MASK);
+
+ return 0;
+}
+
+static const struct imx8mp_blk_ctrl_data imx8mp_media_blk_ctl_dev_data = {
+ .max_reg = 0x138,
+ .probe = imx8mp_media_blk_ctrl_probe,
+ /*
+ * num_domains intentionally omitted (= 0): skip GPC power domain
+ * management for the media blk-ctrl. MEDIAMIX is already powered
+ * on by the boot ROM/SPL, so no GPC sequencing is needed in the
+ * bootloader. With barebox deep-probe enabled, lcdif2
+ * silently ignores the missing genpd provider and probes directly.
+ */
+};
+
static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
{
struct imx8mp_blk_ctrl_domain *domain = to_imx8mp_blk_ctrl_domain(genpd);
@@ -273,12 +344,6 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
return ret;
}
- ret = imx8mp_hsio_propagate_adb_handshake(bc);
- if (ret) {
- dev_err(bc->dev, "failed to propagate adb handshake\n");
- goto bus_put;
- }
-
/* enable upstream clocks */
ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
if (ret) {
@@ -287,7 +352,8 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
}
/* domain specific blk-ctrl manipulation */
- bc->power_on(bc, domain);
+ if (bc->power_on)
+ bc->power_on(bc, domain);
/* power up upstream GPC domain */
ret = pm_runtime_resume_and_get_genpd(domain->power_dev);
@@ -322,7 +388,8 @@ static int imx8mp_blk_ctrl_power_off(struct generic_pm_domain *genpd)
}
/* domain specific blk-ctrl manipulation */
- bc->power_off(bc, domain);
+ if (bc->power_off)
+ bc->power_off(bc, domain);
clk_bulk_disable_unprepare(data->num_clks, domain->clks);
@@ -380,6 +447,35 @@ static int imx8mp_blk_ctrl_probe(struct device *dev)
if (!bc->onecell_data.domains)
return -ENOMEM;
+ /*
+ * Skip GPC power domain management when num_domains == 0.
+ * This is used for blk-ctrl instances (e.g. media) where we only
+ * want the regmap/probe side-effects and not genpd provider
+ * registration, which could trigger GPC power-on sequences at
+ * unexpected times during boot.
+ */
+ if (num_domains == 0) {
+ /*
+ * For the media blk-ctrl we skip genpd provider registration to
+ * avoid triggering full GPC power sequencing for every consumer.
+ * However we still need the MEDIAMIX domain to be powered before
+ * accessing any of its registers (including blk-ctrl itself).
+ * Power it on via the "bus" domain and keep it on.
+ */
+ bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
+ if (!IS_ERR_OR_NULL(bc->bus_power_dev)) {
+ ret = pm_runtime_resume_and_get_genpd(bc->bus_power_dev);
+ if (ret < 0)
+ dev_warn(dev, "failed to power on MEDIAMIX (ignoring): %d\n", ret);
+ }
+ if (bc_data->probe) {
+ ret = bc_data->probe(bc);
+ if (ret)
+ return ret;
+ }
+ return 0;
+ }
+
bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
if (IS_ERR(bc->bus_power_dev))
return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
@@ -461,6 +557,9 @@ static const struct of_device_id imx8mp_blk_ctrl_of_match[] = {
{
.compatible = "fsl,imx8mp-hsio-blk-ctrl",
.data = &imx8mp_hsio_blk_ctl_dev_data,
+ }, {
+ .compatible = "fsl,imx8mp-media-blk-ctrl",
+ .data = &imx8mp_media_blk_ctl_dev_data,
}, {
/* Sentinel */
}
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2/6] pmdomain: imx8mp-blk-ctrl: add media blk-ctrl power domain support
2026-06-02 4:09 ` [PATCH 2/6] pmdomain: imx8mp-blk-ctrl: add media blk-ctrl power domain support Johannes Schneider
@ 2026-06-02 7:34 ` Ahmad Fatoum
0 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2026-06-02 7:34 UTC (permalink / raw)
To: Johannes Schneider, barebox; +Cc: thomas.haemmerle, mfe, Lucas Stach
Hello,
On 6/2/26 6:09 AM, Johannes Schneider wrote:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> The i.MX8MP LCDIF2 controller lives inside the MEDIAMIX power domain,
> which has its own blk-ctrl to gate clocks and control resets. Without
> enabling the MEDIAMIX bus clock and de-asserting the block-level resets
> before the first register access, any read or write to LCDIF2 registers
> causes an AXI bus hang and barebox locks up on startup.
>
> Extend the existing blk-ctrl driver (currently HSIO-only) to support the
> media blk-ctrl (fsl,imx8mp-media-blk-ctrl) by:
> - Separating the HSIO ADB handshake into the HSIO-specific power_on hook
> so the generic path is clean for other blk-ctrls without such handshake.
> - Adding null checks on bc->power_on/off to allow instances without custom
> callbacks.
> - Increasing DOMAIN_MAX_CLKS from 2 to 3 (LCDIF2 needs disp2, axi, apb).
> - Adding media blk-ctrl probe that enables the MEDIAMIX bus clock and
> de-asserts LCDIF2 clocks/resets, mirroring what the Linux kernel does
> in imx8mp_media_power_notifier.
>
> The media_blk_ctrl DTS node retains its syscon compatible so that
> syscon_node_to_regmap() continues to work for the LDB bridge sub-device.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> ---
> drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 117 +++++++++++++++++++++++--
> 1 file changed, 108 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> index 3d302cbde7..87e0114c22 100644
> --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> @@ -17,6 +17,7 @@
>
> #include <dt-bindings/power/imx8mp-power.h>
>
> +/* HSIO blk-ctrl registers */
> #define GPR_REG0 0x0
> #define PCIE_CLOCK_MODULE_EN BIT(0)
> #define USB_CLOCK_MODULE_EN BIT(1)
> @@ -32,6 +33,11 @@
> #define PLL_CKE BIT(17)
> #define PLL_RST BIT(31)
>
> +/* Media blk-ctrl registers */
> +#define LCDIF_ARCACHE_CTRL 0x40
> +#define LCDIF_1_RD_HURRY GENMASK(6, 4)
> +#define LCDIF_0_RD_HURRY GENMASK(2, 0)
> +
> struct imx8mp_blk_ctrl_domain;
>
> struct imx8mp_blk_ctrl {
> @@ -51,7 +57,7 @@ struct imx8mp_blk_ctrl_domain_data {
> const char *gpc_name;
> };
>
> -#define DOMAIN_MAX_CLKS 2
> +#define DOMAIN_MAX_CLKS 3
>
> struct imx8mp_blk_ctrl_domain {
> struct generic_pm_domain genpd;
> @@ -155,9 +161,14 @@ static int imx8mp_hsio_blk_ctrl_probe(struct imx8mp_blk_ctrl *bc)
> return of_clk_add_hw_provider(dev_of_node(bc->dev), of_clk_hw_simple_get, hw);
> }
>
> +static int imx8mp_hsio_propagate_adb_handshake(struct imx8mp_blk_ctrl *bc);
> +
> static void imx8mp_hsio_blk_ctrl_power_on(struct imx8mp_blk_ctrl *bc,
> struct imx8mp_blk_ctrl_domain *domain)
> {
> + /* propagate ADB handshake once before any HSIO sub-domain is enabled */
> + imx8mp_hsio_propagate_adb_handshake(bc);
This used to happen _before_ enabling upstream clocks, but now it
happens _after_. The commit message doesn't explain why this would be ok.
Maybe Marco or Lucas have a thought on this?
Cheers,
Ahmad
> +
> switch (domain->id) {
> case IMX8MP_HSIOBLK_PD_USB:
> regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> @@ -259,6 +270,66 @@ static const struct imx8mp_blk_ctrl_data imx8mp_hsio_blk_ctl_dev_data = {
> .num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data),
> };
>
> +/* Media blk-ctrl */
> +
> +/*
> + * MEDIAMIX BLK_CTRL register offsets (from i.MX 8M Plus RM, section 13).
> + * Bit SET = reset de-asserted / clock enabled.
> + */
> +#define BLK_SFT_RSTN 0x00
> +#define BLK_CLK_EN 0x04
> +
> +/* BIT(8): bus/APB clock and reset for the MEDIAMIX interconnect */
> +#define MEDIAMIX_BUS_CLK_RST BIT(8)
> +
> +/*
> + * LCDIF2 (mediablk-lcdif-2) bits in BLK_SFT_RSTN and BLK_CLK_EN:
> + * BIT(11): lcdif2-axi, BIT(12): lcdif2-apb, BIT(24): disp2 pixel clock
> + * (from Linux kernel imx8m-blk-ctrl.c IMX8MP_MEDIABLK_PD_LCDIF_2)
> + */
> +#define LCDIF2_CLK_RST_MASK (BIT(11) | BIT(12) | BIT(24))
> +
> +static int imx8mp_media_blk_ctrl_probe(struct imx8mp_blk_ctrl *bc)
> +{
> + /* Set panic read hurry level for LCDIF interfaces to 7 */
> + regmap_update_bits(bc->regmap, LCDIF_ARCACHE_CTRL,
> + FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
> + FIELD_PREP(LCDIF_0_RD_HURRY, 7),
> + FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
> + FIELD_PREP(LCDIF_0_RD_HURRY, 7));
> +
> + /*
> + * Enable the MEDIAMIX bus clock and de-assert its reset so the
> + * internal AHB/APB fabric is up before sub-module access.
> + * (Mirrors Linux kernel imx8mp_media_power_notifier BIT(8) writes.)
> + */
> + regmap_set_bits(bc->regmap, BLK_CLK_EN, MEDIAMIX_BUS_CLK_RST);
> + regmap_set_bits(bc->regmap, BLK_SFT_RSTN, MEDIAMIX_BUS_CLK_RST);
> + udelay(5); /* wait for ADB handshake, as Linux kernel does */
> +
> + /*
> + * Enable LCDIF2 clocks and de-assert its reset within MEDIAMIX.
> + * Without this, LCDIF2 registers are inaccessible (AXI bus hangs).
> + * (Mirrors Linux kernel imx8mp_blk_ctrl_power_on for LCDIF_2 domain.)
> + */
> + regmap_set_bits(bc->regmap, BLK_CLK_EN, LCDIF2_CLK_RST_MASK);
> + regmap_set_bits(bc->regmap, BLK_SFT_RSTN, LCDIF2_CLK_RST_MASK);
> +
> + return 0;
> +}
> +
> +static const struct imx8mp_blk_ctrl_data imx8mp_media_blk_ctl_dev_data = {
> + .max_reg = 0x138,
> + .probe = imx8mp_media_blk_ctrl_probe,
> + /*
> + * num_domains intentionally omitted (= 0): skip GPC power domain
> + * management for the media blk-ctrl. MEDIAMIX is already powered
> + * on by the boot ROM/SPL, so no GPC sequencing is needed in the
> + * bootloader. With barebox deep-probe enabled, lcdif2
> + * silently ignores the missing genpd provider and probes directly.
> + */
> +};
> +
> static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> {
> struct imx8mp_blk_ctrl_domain *domain = to_imx8mp_blk_ctrl_domain(genpd);
> @@ -273,12 +344,6 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> return ret;
> }
>
> - ret = imx8mp_hsio_propagate_adb_handshake(bc);
> - if (ret) {
> - dev_err(bc->dev, "failed to propagate adb handshake\n");
> - goto bus_put;
> - }
> -
> /* enable upstream clocks */
> ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> if (ret) {
> @@ -287,7 +352,8 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> }
>
> /* domain specific blk-ctrl manipulation */
> - bc->power_on(bc, domain);
> + if (bc->power_on)
> + bc->power_on(bc, domain);
>
> /* power up upstream GPC domain */
> ret = pm_runtime_resume_and_get_genpd(domain->power_dev);
> @@ -322,7 +388,8 @@ static int imx8mp_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> }
>
> /* domain specific blk-ctrl manipulation */
> - bc->power_off(bc, domain);
> + if (bc->power_off)
> + bc->power_off(bc, domain);
>
> clk_bulk_disable_unprepare(data->num_clks, domain->clks);
>
> @@ -380,6 +447,35 @@ static int imx8mp_blk_ctrl_probe(struct device *dev)
> if (!bc->onecell_data.domains)
> return -ENOMEM;
>
> + /*
> + * Skip GPC power domain management when num_domains == 0.
> + * This is used for blk-ctrl instances (e.g. media) where we only
> + * want the regmap/probe side-effects and not genpd provider
> + * registration, which could trigger GPC power-on sequences at
> + * unexpected times during boot.
> + */
> + if (num_domains == 0) {
> + /*
> + * For the media blk-ctrl we skip genpd provider registration to
> + * avoid triggering full GPC power sequencing for every consumer.
> + * However we still need the MEDIAMIX domain to be powered before
> + * accessing any of its registers (including blk-ctrl itself).
> + * Power it on via the "bus" domain and keep it on.
> + */
> + bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
> + if (!IS_ERR_OR_NULL(bc->bus_power_dev)) {
> + ret = pm_runtime_resume_and_get_genpd(bc->bus_power_dev);
> + if (ret < 0)
> + dev_warn(dev, "failed to power on MEDIAMIX (ignoring): %d\n", ret);
> + }
> + if (bc_data->probe) {
> + ret = bc_data->probe(bc);
> + if (ret)
> + return ret;
> + }
> + return 0;
> + }
> +
> bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
> if (IS_ERR(bc->bus_power_dev))
> return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> @@ -461,6 +557,9 @@ static const struct of_device_id imx8mp_blk_ctrl_of_match[] = {
> {
> .compatible = "fsl,imx8mp-hsio-blk-ctrl",
> .data = &imx8mp_hsio_blk_ctl_dev_data,
> + }, {
> + .compatible = "fsl,imx8mp-media-blk-ctrl",
> + .data = &imx8mp_media_blk_ctl_dev_data,
> }, {
> /* Sentinel */
> }
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/6] video: backlight-pwm: make power-supply and enable-gpio optional
2026-06-02 4:09 [PATCH 0/7] video: enable boot splash on i.MX8MP with LVDS panel Johannes Schneider
2026-06-02 4:09 ` [PATCH 1/6] clk: imx8mp: add 700 MHz rate entry for VIDEO_PLL1 Johannes Schneider
2026-06-02 4:09 ` [PATCH 2/6] pmdomain: imx8mp-blk-ctrl: add media blk-ctrl power domain support Johannes Schneider
@ 2026-06-02 4:09 ` Johannes Schneider
2026-06-02 7:22 ` Ahmad Fatoum
2026-06-02 4:09 ` [PATCH 4/6] video: add i.MX8MP LCDIF2 V8 framebuffer driver Johannes Schneider
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schneider @ 2026-06-02 4:09 UTC (permalink / raw)
To: barebox; +Cc: thomas.haemmerle
From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
PWM backlight DT nodes for board displays often omit the power-supply regulator
(the rail is always-on) or use enable-gpios without a supply. The driver
previously treated both as mandatory and returned -ENODEV if either was absent,
blocking display initialisation on devices which use an always-on 3.3V supply
and no separate backlight enable regulator.
Guard the regulator enable/disable calls with a NULL check, handle
-ENODEV from regulator_get() as "no supply present", and use GPIOD_ASIS
for enable GPIO so the initial pin state is not disturbed.
Also improve the "Cannot find PWM device" error message to print the
actual error pointer so probe failures are diagnosable.
Assisted-by: Claude:claude-sonnet-4-6
Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
---
drivers/video/backlight-pwm.c | 16 ++++++++++------
drivers/video/backlight.c | 3 ++-
drivers/video/fsl-ldb.c | 2 +-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/video/backlight-pwm.c b/drivers/video/backlight-pwm.c
index d3c81114e0..9c8fcfbe93 100644
--- a/drivers/video/backlight-pwm.c
+++ b/drivers/video/backlight-pwm.c
@@ -37,7 +37,8 @@ static int backlight_pwm_enable(struct pwm_backlight *pwm_backlight)
if (ret)
return ret;
- regulator_enable(pwm_backlight->power);
+ if (pwm_backlight->power)
+ regulator_enable(pwm_backlight->power);
gpiod_direction_output(pwm_backlight->enable_gpio, true);
@@ -55,7 +56,8 @@ static int backlight_pwm_disable(struct pwm_backlight *pwm_backlight)
ret = gpiod_direction_output(pwm_backlight->enable_gpio, false);
if (!ret) {
- regulator_disable(pwm_backlight->power);
+ if (pwm_backlight->power)
+ regulator_disable(pwm_backlight->power);
/*
* Only disable PWM when an enable gpio is present.
@@ -148,7 +150,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
pwm_backlight->backlight.brightness_max = pwm_backlight->scale;
}
- pwm_backlight->enable_gpio = gpiod_get_optional(dev, "enable-gpios", 0);
+ pwm_backlight->enable_gpio = gpiod_get_optional(dev, "enable", GPIOD_ASIS);
return 0;
}
@@ -161,7 +163,7 @@ static int backlight_pwm_of_probe(struct device *dev)
pwm = of_pwm_request(dev->of_node, NULL);
if (IS_ERR(pwm)) {
- dev_err(dev, "Cannot find PWM device\n");
+ dev_err(dev, "Cannot find PWM device: %pe\n", pwm);
return PTR_ERR(pwm);
}
@@ -175,8 +177,10 @@ static int backlight_pwm_of_probe(struct device *dev)
pwm_backlight->power = regulator_get(dev, "power");
if (IS_ERR(pwm_backlight->power)) {
- dev_err(dev, "Cannot find regulator\n");
- return PTR_ERR(pwm_backlight->power);
+ if (PTR_ERR(pwm_backlight->power) != -ENODEV)
+ return dev_errp_probe(dev, pwm_backlight->power,
+ "power supply\n");
+ pwm_backlight->power = NULL;
}
pwm_backlight->backlight.slew_time_ms = 100;
diff --git a/drivers/video/backlight.c b/drivers/video/backlight.c
index 6d8146ee5a..bf7d40b59a 100644
--- a/drivers/video/backlight.c
+++ b/drivers/video/backlight.c
@@ -94,8 +94,9 @@ int backlight_register(struct backlight_device *bl)
struct backlight_device *of_backlight_find(struct device_node *node)
{
struct backlight_device *bl;
+ int ret;
- of_device_ensure_probed(node);
+ ret = of_device_ensure_probed(node);
class_for_each_container_of_device(&backlight_class, bl, dev)
if (bl->node == node)
diff --git a/drivers/video/fsl-ldb.c b/drivers/video/fsl-ldb.c
index 0ab720032c..09804ac329 100644
--- a/drivers/video/fsl-ldb.c
+++ b/drivers/video/fsl-ldb.c
@@ -287,7 +287,7 @@ static int fsl_ldb_probe(struct device *dev)
/* Only DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS is supported */
- dev_info(dev, "LVDS channel pixel swap not supported.\n");
+ dev_dbg(dev, "LVDS channel pixel swap not supported.\n");
fsl_ldb->vpl.node = dev->of_node;
fsl_ldb->vpl.ioctl = &fsl_ldb_ioctl;
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/6] video: backlight-pwm: make power-supply and enable-gpio optional
2026-06-02 4:09 ` [PATCH 3/6] video: backlight-pwm: make power-supply and enable-gpio optional Johannes Schneider
@ 2026-06-02 7:22 ` Ahmad Fatoum
0 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2026-06-02 7:22 UTC (permalink / raw)
To: Johannes Schneider, barebox; +Cc: thomas.haemmerle
Hello,
Thanks for your patch.
It fixes a real bug, but I have some comments below.
On 6/2/26 6:09 AM, Johannes Schneider wrote:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> PWM backlight DT nodes for board displays often omit the power-supply regulator
> (the rail is always-on) or use enable-gpios without a supply. The driver
> previously treated both as mandatory and returned -ENODEV if either was absent,
> blocking display initialisation on devices which use an always-on 3.3V supply
> and no separate backlight enable regulator.
>
> Guard the regulator enable/disable calls with a NULL check, handle
> -ENODEV from regulator_get() as "no supply present", and use GPIOD_ASIS
> for enable GPIO so the initial pin state is not disturbed.
>
> Also improve the "Cannot find PWM device" error message to print the
> actual error pointer so probe failures are diagnosable.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> ---
> drivers/video/backlight-pwm.c | 16 ++++++++++------
> drivers/video/backlight.c | 3 ++-
> drivers/video/fsl-ldb.c | 2 +-
> 3 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/backlight-pwm.c b/drivers/video/backlight-pwm.c
> index d3c81114e0..9c8fcfbe93 100644
> --- a/drivers/video/backlight-pwm.c
> +++ b/drivers/video/backlight-pwm.c
> @@ -37,7 +37,8 @@ static int backlight_pwm_enable(struct pwm_backlight *pwm_backlight)
> if (ret)
> return ret;
>
> - regulator_enable(pwm_backlight->power);
> + if (pwm_backlight->power)
> + regulator_enable(pwm_backlight->power);
Not needed, regulator_enable gracefully handles a NULL pointer.
>
> gpiod_direction_output(pwm_backlight->enable_gpio, true);
>
> @@ -55,7 +56,8 @@ static int backlight_pwm_disable(struct pwm_backlight *pwm_backlight)
>
> ret = gpiod_direction_output(pwm_backlight->enable_gpio, false);
> if (!ret) {
> - regulator_disable(pwm_backlight->power);
> + if (pwm_backlight->power)
> + regulator_disable(pwm_backlight->power);
Likewise.
>
> /*
> * Only disable PWM when an enable gpio is present.
> @@ -148,7 +150,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
> pwm_backlight->backlight.brightness_max = pwm_backlight->scale;
> }
>
> - pwm_backlight->enable_gpio = gpiod_get_optional(dev, "enable-gpios", 0);
> + pwm_backlight->enable_gpio = gpiod_get_optional(dev, "enable", GPIOD_ASIS);
While the commit message doesn't point it out, use of enable-gpios here
is a bug.
Fixes: 4c7238df6866 ("video: backlight-pwm: switch to gpiod functions")
>
> return 0;
> }
> @@ -161,7 +163,7 @@ static int backlight_pwm_of_probe(struct device *dev)
>
> pwm = of_pwm_request(dev->of_node, NULL);
> if (IS_ERR(pwm)) {
> - dev_err(dev, "Cannot find PWM device\n");
> + dev_err(dev, "Cannot find PWM device: %pe\n", pwm);
> return PTR_ERR(pwm);
return dev_errp_probe(dev, pwm, "Cannot find PWM device\n");
> }
>
> @@ -175,8 +177,10 @@ static int backlight_pwm_of_probe(struct device *dev)
>
> pwm_backlight->power = regulator_get(dev, "power");
regulator_get_optional()
> if (IS_ERR(pwm_backlight->power)) {
> - dev_err(dev, "Cannot find regulator\n");
> - return PTR_ERR(pwm_backlight->power);
> + if (PTR_ERR(pwm_backlight->power) != -ENODEV)
> + return dev_errp_probe(dev, pwm_backlight->power,
> + "power supply\n");
> + pwm_backlight->power = NULL;
> }
>
> pwm_backlight->backlight.slew_time_ms = 100;
> diff --git a/drivers/video/backlight.c b/drivers/video/backlight.c
> index 6d8146ee5a..bf7d40b59a 100644
> --- a/drivers/video/backlight.c
> +++ b/drivers/video/backlight.c
> @@ -94,8 +94,9 @@ int backlight_register(struct backlight_device *bl)
> struct backlight_device *of_backlight_find(struct device_node *node)
> {
> struct backlight_device *bl;
> + int ret;
>
> - of_device_ensure_probed(node);
> + ret = of_device_ensure_probed(node);
ret is never evaluated, so this change serves no purpose.
It's also not needed, because the iteration below will either find a
backlight or it won't find one. You can equally well just cast this to
(void) here.
>
> class_for_each_container_of_device(&backlight_class, bl, dev)
> if (bl->node == node)
> diff --git a/drivers/video/fsl-ldb.c b/drivers/video/fsl-ldb.c
> index 0ab720032c..09804ac329 100644
> --- a/drivers/video/fsl-ldb.c
> +++ b/drivers/video/fsl-ldb.c
> @@ -287,7 +287,7 @@ static int fsl_ldb_probe(struct device *dev)
>
> /* Only DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS is supported */
>
> - dev_info(dev, "LVDS channel pixel swap not supported.\n");
> + dev_dbg(dev, "LVDS channel pixel swap not supported.\n");
Cheers,
Ahmad
>
> fsl_ldb->vpl.node = dev->of_node;
> fsl_ldb->vpl.ioctl = &fsl_ldb_ioctl;
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/6] video: add i.MX8MP LCDIF2 V8 framebuffer driver
2026-06-02 4:09 [PATCH 0/7] video: enable boot splash on i.MX8MP with LVDS panel Johannes Schneider
` (2 preceding siblings ...)
2026-06-02 4:09 ` [PATCH 3/6] video: backlight-pwm: make power-supply and enable-gpio optional Johannes Schneider
@ 2026-06-02 4:09 ` Johannes Schneider
2026-06-02 7:16 ` Ahmad Fatoum
2026-06-02 4:09 ` [PATCH 5/6] video: add generic panel-lvds driver Johannes Schneider
2026-06-02 4:09 ` [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels Johannes Schneider
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schneider @ 2026-06-02 4:09 UTC (permalink / raw)
To: barebox; +Cc: thomas.haemmerle
From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
The i.MX8MP has a new LCDIF V8 display controller with a different
register layout from the older mxsfb-based LCDIF found on i.MX6/7/8MM.
The existing DRIVER_VIDEO_LCDIF driver does not support this variant,
leaving the i.MX8MP without a barebox framebuffer and no way to show a
boot splash.
The driver:
- Programs LCDIF V8 timing registers for the configured video mode.
- Uses 128B DMA burst size (P_SIZE=1, T_SIZE=1) so that 800-pixel rows
(3200 bytes) divide into exactly 25 complete bursts; 256B bursts leave
a partial burst and produce a ~32px black strip at the right edge.
- Sets correct CTRL polarity bit positions (INV_HS=bit0, INV_VS=bit1,
INV_DE=bit2, INV_PXCK=bit3); the i.MX8MP TRM places these at bits 0-3,
not 2-5 as a naive port from mxsfb would suggest.
- Allocates a write-combine DMA framebuffer and registers a simplefb DT
fixup for Linux DRM_SIMPLEDRM to inherit the boot image.
- Enables the framebuffer at probe time so the splash command only needs
to blit pixels; fb_enable() is not called per-splash.
Assisted-by: Claude:claude-sonnet-4-6
Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
---
drivers/video/Kconfig | 10 +
drivers/video/Makefile | 1 +
drivers/video/imx-lcdif.c | 378 ++++++++++++++++++++++++++++++++++++++
3 files changed, 389 insertions(+)
create mode 100644 drivers/video/imx-lcdif.c
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index ce10237221..fbcec6fd67 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -194,6 +194,16 @@ config DRIVER_VIDEO_TC358767
help
The TC358767A is a DSI/DPI to eDP video encoder chip
+config DRIVER_VIDEO_IMX_LCDIF
+ bool "i.MX8MP LCDIF V8 framebuffer driver"
+ select VIDEO_VPL
+ select DRIVER_VIDEO_SIMPLEFB
+ depends on OFTREE && OFDEVICE
+ help
+ Framebuffer driver for the i.MX8MP/i.MX93 LCDIF V8 display controller.
+ Supports the "fsl,imx8mp-lcdif" compatible. Creates a simple-framebuffer
+ device tree node for kernel handoff via DRM_SIMPLEDRM.
+
config DRIVER_VIDEO_SIMPLE_PANEL
bool "Simple panel support"
select VIDEO_VPL
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 7b10eda0d8..9957ff5ad2 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbconsole.o
obj-$(CONFIG_VIDEO_VPL) += vpl.o
obj-$(CONFIG_DRIVER_VIDEO_MTL017) += mtl017.o
obj-$(CONFIG_DRIVER_VIDEO_TC358767) += tc358767.o
+obj-$(CONFIG_DRIVER_VIDEO_IMX_LCDIF) += imx-lcdif.o
obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_PANEL) += simple-panel.o
obj-$(CONFIG_DRIVER_VIDEO_MIPI_DBI) += mipi_dbi.o
obj-$(CONFIG_DRIVER_VIDEO_MIPI_DSI) += mipi_dsi.o
diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
new file mode 100644
index 0000000000..ae5976c771
--- /dev/null
+++ b/drivers/video/imx-lcdif.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * i.MX8MP LCDIF V8 framebuffer driver for barebox
+ *
+ * Based on Linux drivers/gpu/drm/mxsfb/lcdif_drv.c and lcdif_kms.c
+ * Copyright (C) 2022 Marek Vasut <marex@denx.de>
+ *
+ * Copyright Leica Geosystems AG
+ */
+
+#include <common.h>
+#include <init.h>
+#include <driver.h>
+#include <io.h>
+#include <dma.h>
+#include <fb.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <of_graph.h>
+#include <video/media-bus-format.h>
+#include <video/vpl.h>
+
+/* LCDIF V8 register map */
+#define LCDC_V8_CTRL 0x00
+#define LCDC_V8_DISP_PARA 0x10
+#define LCDC_V8_DISP_SIZE 0x14
+#define LCDC_V8_HSYN_PARA 0x18
+#define LCDC_V8_VSYN_PARA 0x1c
+#define LCDC_V8_VSYN_HSYN_WIDTH 0x20
+#define LCDC_V8_INT_STATUS_D0 0x24
+#define LCDC_V8_INT_ENABLE_D0 0x28
+#define LCDC_V8_INT_STATUS_D1 0x30
+#define LCDC_V8_INT_ENABLE_D1 0x34
+#define LCDC_V8_CTRLDESCL0_1 0x200
+#define LCDC_V8_CTRLDESCL0_3 0x208
+#define LCDC_V8_CTRLDESCL_LOW0_4 0x20c
+#define LCDC_V8_CTRLDESCL_HIGH0_4 0x210
+#define LCDC_V8_CTRLDESCL0_5 0x214
+#define LCDC_V8_CSC0_CTRL 0x21c
+#define LCDC_V8_PANIC0_THRES 0x238
+
+/* CTRL bits */
+#define CTRL_SW_RESET BIT(31)
+#define CTRL_FETCH_START_OPTION_FPV 0
+#define CTRL_INV_HS BIT(0)
+#define CTRL_INV_VS BIT(1)
+#define CTRL_INV_DE BIT(2)
+#define CTRL_INV_PXCK BIT(3)
+
+/* DISP_PARA bits */
+#define DISP_PARA_DISP_ON BIT(31)
+#define DISP_PARA_LINE_PATTERN_RGB888 (0x0 << 26)
+#define DISP_PARA_LINE_PATTERN_UYVY (0x3 << 26)
+#define DISP_PARA_LINE_PATTERN_MASK GENMASK(29, 26)
+
+/* DISP_SIZE: Y[31:16] X[15:0] */
+#define DISP_SIZE_DELTA_Y(y) (((y) & 0xffff) << 16)
+#define DISP_SIZE_DELTA_X(x) ((x) & 0xffff)
+
+/* HSYN_PARA: back_porch[31:16] front_porch[15:0] */
+#define HSYN_PARA_BP_H(bp) (((bp) & 0xffff) << 16)
+#define HSYN_PARA_FP_H(fp) ((fp) & 0xffff)
+
+/* VSYN_PARA: back_porch[31:16] front_porch[15:0] */
+#define VSYN_PARA_BP_V(bp) (((bp) & 0xffff) << 16)
+#define VSYN_PARA_FP_V(fp) ((fp) & 0xffff)
+
+/* VSYN_HSYN_WIDTH: vsync[31:16] hsync[15:0] */
+#define VSYN_HSYN_WIDTH_PW_V(v) (((v) & 0xffff) << 16)
+#define VSYN_HSYN_WIDTH_PW_H(h) ((h) & 0xffff)
+
+/* CTRLDESCL0_1: height[31:16] width[15:0] */
+#define CTRLDESCL0_1_HEIGHT(h) (((h) & 0xffff) << 16)
+#define CTRLDESCL0_1_WIDTH(w) ((w) & 0xffff)
+
+/* CTRLDESCL0_3: P_SIZE[22:20] T_SIZE[17:16] PITCH[15:0] */
+#define CTRLDESCL0_3_P_SIZE(s) (((s) & 0x7) << 20)
+#define CTRLDESCL0_3_T_SIZE(s) (((s) & 0x3) << 16)
+#define CTRLDESCL0_3_PITCH(p) ((p) & 0xffff)
+
+/* CTRLDESCL0_5 */
+#define CTRLDESCL0_5_EN BIT(31)
+#define CTRLDESCL0_5_SHADOW_LOAD_EN BIT(30)
+#define CTRLDESCL0_5_BPP_16_RGB565 (0x4 << 24)
+#define CTRLDESCL0_5_BPP_24_RGB888 (0x8 << 24)
+#define CTRLDESCL0_5_BPP_32_ARGB8888 (0x9 << 24)
+#define CTRLDESCL0_5_BPP_MASK GENMASK(27, 24)
+
+/* CSC0_CTRL */
+#define CSC0_CTRL_BYPASS BIT(0)
+
+/* INT_ENABLE_D1 */
+#define INT_ENABLE_D1_PLANE_PANIC_EN BIT(0)
+
+/* PANIC0_THRES */
+#define PANIC0_THRES_LOW_MASK GENMASK(8, 0)
+#define PANIC0_THRES_HIGH_MASK GENMASK(24, 16)
+#define PANIC0_THRES_MAX 511
+
+struct lcdif_priv {
+ void __iomem *base;
+ struct clk *clk_pix;
+ struct clk *clk_axi;
+ struct clk *clk_disp_axi;
+ struct fb_info info;
+ struct vpl vpl;
+ int port_id;
+};
+
+static void lcdif_reset(struct lcdif_priv *priv)
+{
+ writel(CTRL_SW_RESET, priv->base + LCDC_V8_CTRL);
+ udelay(10);
+ writel(0, priv->base + LCDC_V8_CTRL);
+}
+
+static void lcdif_set_mode(struct lcdif_priv *priv, struct fb_videomode *mode)
+{
+ u32 ctrl = 0;
+
+ if (mode->sync & FB_SYNC_HOR_HIGH_ACT)
+ ; /* HSYNC active high = no invert */
+ else
+ ctrl |= CTRL_INV_HS;
+
+ if (mode->sync & FB_SYNC_VERT_HIGH_ACT)
+ ; /* VSYNC active high = no invert */
+ else
+ ctrl |= CTRL_INV_VS;
+
+ if (mode->display_flags & DISPLAY_FLAGS_DE_LOW)
+ ctrl |= CTRL_INV_DE;
+ if (mode->display_flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+ ctrl |= CTRL_INV_PXCK;
+
+ writel(ctrl, priv->base + LCDC_V8_CTRL);
+
+ writel(DISP_SIZE_DELTA_Y(mode->yres) | DISP_SIZE_DELTA_X(mode->xres),
+ priv->base + LCDC_V8_DISP_SIZE);
+
+ /* hback_porch = htotal - hsync_end, hfront_porch = hsync_start - hactive */
+ writel(HSYN_PARA_BP_H(mode->left_margin) | HSYN_PARA_FP_H(mode->right_margin),
+ priv->base + LCDC_V8_HSYN_PARA);
+
+ writel(VSYN_PARA_BP_V(mode->upper_margin) | VSYN_PARA_FP_V(mode->lower_margin),
+ priv->base + LCDC_V8_VSYN_PARA);
+
+ writel(VSYN_HSYN_WIDTH_PW_V(mode->vsync_len) | VSYN_HSYN_WIDTH_PW_H(mode->hsync_len),
+ priv->base + LCDC_V8_VSYN_HSYN_WIDTH);
+
+ writel(CTRLDESCL0_1_HEIGHT(mode->yres) | CTRLDESCL0_1_WIDTH(mode->xres),
+ priv->base + LCDC_V8_CTRLDESCL0_1);
+}
+
+static void lcdif_set_plane(struct lcdif_priv *priv)
+{
+ struct fb_info *info = &priv->info;
+ phys_addr_t paddr = virt_to_phys(info->screen_base);
+ u32 ctrl;
+ u32 stride = info->line_length;
+
+ /*
+ * AXI burst size: P_SIZE=1 = 128B, T_SIZE=1 = 128B threshold.
+ * Use 128B (32-pixel @ 32bpp) bursts so that 800-pixel rows
+ * (3200 bytes) divide evenly: 3200 / 128 = 25 complete bursts.
+ * P_SIZE=2 (256B) would leave a partial burst (3200/256 = 12.5)
+ * causing a ~32-pixel dark gap at the right edge of the display.
+ */
+ ctrl = CTRLDESCL0_3_P_SIZE(1) | CTRLDESCL0_3_T_SIZE(1) |
+ CTRLDESCL0_3_PITCH(stride);
+ writel(ctrl, priv->base + LCDC_V8_CTRLDESCL0_3);
+
+ writel(lower_32_bits(paddr), priv->base + LCDC_V8_CTRLDESCL_LOW0_4);
+ writel(upper_32_bits(paddr), priv->base + LCDC_V8_CTRLDESCL_HIGH0_4);
+
+ /* 32bpp XRGB8888 → ARGB8888 pixel format.
+ * SHADOW_LOAD_EN causes the descriptor registers (address, format, pitch)
+ * to be latched into the active registers on the next VSYNC, which is
+ * required for the initial descriptor load in barebox's single-enable
+ * sequence (no DRM page-flip mechanism). EN is ORed in later by
+ * lcdif_enable() after DISP_ON. */
+ writel(CTRLDESCL0_5_BPP_32_ARGB8888 | CTRLDESCL0_5_SHADOW_LOAD_EN,
+ priv->base + LCDC_V8_CTRLDESCL0_5);
+}
+
+static void lcdif_enable(struct lcdif_priv *priv)
+{
+ u32 reg;
+
+ /* Set FIFO Panic watermarks: low=1/3, high=2/3 */
+ writel(FIELD_PREP(PANIC0_THRES_LOW_MASK, 1 * PANIC0_THRES_MAX / 3) |
+ FIELD_PREP(PANIC0_THRES_HIGH_MASK, 2 * PANIC0_THRES_MAX / 3),
+ priv->base + LCDC_V8_PANIC0_THRES);
+
+ writel(INT_ENABLE_D1_PLANE_PANIC_EN, priv->base + LCDC_V8_INT_ENABLE_D1);
+
+ /* Write LINE_PATTERN_RGB888 (= 0) first to clear all DISP_PARA bits,
+ * including BGND_R/G/B (bits [23:0]) — ensures background is black. */
+ writel(DISP_PARA_LINE_PATTERN_RGB888, priv->base + LCDC_V8_DISP_PARA);
+ reg = readl(priv->base + LCDC_V8_DISP_PARA);
+ reg |= DISP_PARA_DISP_ON;
+ writel(reg, priv->base + LCDC_V8_DISP_PARA);
+
+ reg = readl(priv->base + LCDC_V8_CTRLDESCL0_5);
+ reg |= CTRLDESCL0_5_EN;
+ writel(reg, priv->base + LCDC_V8_CTRLDESCL0_5);
+}
+
+static int lcdif_fb_enable(struct fb_info *info)
+{
+ struct lcdif_priv *priv = info->priv;
+ struct fb_videomode *mode = info->mode;
+ int ret;
+
+ clk_set_rate(priv->clk_pix, PICOS2KHZ(mode->pixclock) * 1000UL);
+ clk_prepare_enable(priv->clk_axi);
+ clk_prepare_enable(priv->clk_disp_axi);
+ clk_prepare_enable(priv->clk_pix);
+
+ /* Notify downstream bridge about mode and enable it */
+ ret = vpl_ioctl_prepare(&priv->vpl, priv->port_id, mode);
+ if (ret) {
+ dev_err(info->dev.parent, "vpl_prepare failed: %pe\n", ERR_PTR(ret));
+ return ret;
+ }
+ lcdif_reset(priv);
+ lcdif_set_mode(priv, mode);
+ lcdif_set_plane(priv);
+
+ /* Bypass CSC for RGB input */
+ writel(CSC0_CTRL_BYPASS, priv->base + LCDC_V8_CSC0_CTRL);
+
+ lcdif_enable(priv);
+
+ ret = vpl_ioctl_enable(&priv->vpl, priv->port_id);
+ if (ret)
+ dev_warn(info->dev.parent, "vpl_enable failed: %pe\n", ERR_PTR(ret));
+
+ return 0;
+}
+
+static void lcdif_fb_disable(struct fb_info *info)
+{
+ struct lcdif_priv *priv = info->priv;
+ u32 reg;
+
+ vpl_ioctl_disable(&priv->vpl, priv->port_id);
+
+ reg = readl(priv->base + LCDC_V8_CTRLDESCL0_5);
+ reg &= ~CTRLDESCL0_5_EN;
+ writel(reg, priv->base + LCDC_V8_CTRLDESCL0_5);
+
+ reg = readl(priv->base + LCDC_V8_DISP_PARA);
+ reg &= ~DISP_PARA_DISP_ON;
+ writel(reg, priv->base + LCDC_V8_DISP_PARA);
+
+ vpl_ioctl_unprepare(&priv->vpl, priv->port_id);
+
+ clk_disable_unprepare(priv->clk_pix);
+ clk_disable_unprepare(priv->clk_disp_axi);
+ clk_disable_unprepare(priv->clk_axi);
+}
+
+static struct fb_ops lcdif_fb_ops = {
+ .fb_enable = lcdif_fb_enable,
+ .fb_disable = lcdif_fb_disable,
+};
+
+static int lcdif_probe(struct device *dev)
+{
+ struct resource *iores;
+ struct lcdif_priv *priv;
+ struct fb_info *info;
+ int ret;
+
+ iores = dev_request_mem_resource(dev, 0);
+ if (IS_ERR(iores))
+ return PTR_ERR(iores);
+
+ priv = xzalloc(sizeof(*priv));
+ priv->base = IOMEM(iores->start);
+
+ priv->clk_pix = clk_get(dev, "pix");
+ if (IS_ERR(priv->clk_pix))
+ return dev_errp_probe(dev, priv->clk_pix, "pixel clock\n");
+
+ priv->clk_axi = clk_get(dev, "axi");
+ if (IS_ERR(priv->clk_axi))
+ return dev_errp_probe(dev, priv->clk_axi, "axi clock\n");
+
+ priv->clk_disp_axi = clk_get(dev, "disp_axi");
+ if (IS_ERR(priv->clk_disp_axi))
+ return dev_errp_probe(dev, priv->clk_disp_axi, "disp_axi clock\n");
+
+ /* Register VPL node - endpoint is port@0 */
+ priv->port_id = 0;
+ priv->vpl.node = dev->of_node;
+ ret = vpl_register(&priv->vpl);
+ if (ret)
+ return ret;
+
+ info = &priv->info;
+ info->priv = priv;
+ info->fbops = &lcdif_fb_ops;
+
+ /* Fixed 32bpp XRGB8888 format */
+ info->bits_per_pixel = 32;
+ info->red = (struct fb_bitfield){ .offset = 16, .length = 8 };
+ info->green = (struct fb_bitfield){ .offset = 8, .length = 8 };
+ info->blue = (struct fb_bitfield){ .offset = 0, .length = 8 };
+ info->transp = (struct fb_bitfield){ .offset = 24, .length = 8 };
+
+ /* Get display modes from downstream panel via VPL */
+ ret = vpl_ioctl(&priv->vpl, priv->port_id, VPL_GET_VIDEOMODES, &info->modes);
+ if (ret) {
+ dev_dbg(dev, "no modes from VPL: %pe\n", ERR_PTR(ret));
+ return ret;
+ }
+
+ /* Allocate DMA-coherent framebuffer */
+ if (info->modes.num_modes > 0) {
+ struct fb_videomode *m = &info->modes.modes[0];
+ size_t fb_size = m->xres * m->yres * (info->bits_per_pixel / 8);
+
+ info->screen_base = dma_alloc_writecombine(DMA_DEVICE_BROKEN,
+ fb_size,
+ DMA_ADDRESS_BROKEN);
+ if (!info->screen_base) {
+ dev_err(dev, "failed to allocate framebuffer\n");
+ return -ENOMEM;
+ }
+ info->screen_size = fb_size;
+ memset(info->screen_base, 0, fb_size);
+ }
+
+ info->dev.parent = dev;
+ ret = register_framebuffer(info);
+ if (ret) {
+ dev_err(dev, "failed to register framebuffer: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Create simple-framebuffer DT node for Linux kernel handoff.
+ * With CONFIG_DRM_SIMPLEDRM, Linux will inherit the barebox
+ * framebuffer and keep the boot splash visible until platsch
+ * renders via the LCDIF DRM driver.
+ */
+ info->register_simplefb = 1;
+ fb_register_simplefb(info);
+
+ /*
+ * Enable the display immediately so the framebuffer is live.
+ * The splash command only blits pixels to the buffer - it does not
+ * call fb_enable itself - so we must enable here during probe.
+ */
+ ret = fb_enable(info);
+ if (ret)
+ dev_warn(dev, "failed to enable framebuffer: %d\n", ret);
+
+ dev_info(dev, "i.MX8MP LCDIF registered\n");
+ return 0;
+}
+
+static const struct of_device_id lcdif_dt_ids[] = {
+ { .compatible = "fsl,imx8mp-lcdif" },
+ { .compatible = "fsl,imx93-lcdif" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, lcdif_dt_ids);
+
+static struct driver lcdif_driver = {
+ .name = "imx-lcdif",
+ .probe = lcdif_probe,
+ .of_compatible = DRV_OF_COMPAT(lcdif_dt_ids),
+};
+device_platform_driver(lcdif_driver);
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/6] video: add i.MX8MP LCDIF2 V8 framebuffer driver
2026-06-02 4:09 ` [PATCH 4/6] video: add i.MX8MP LCDIF2 V8 framebuffer driver Johannes Schneider
@ 2026-06-02 7:16 ` Ahmad Fatoum
2026-06-04 3:34 ` SCHNEIDER Johannes
0 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2026-06-02 7:16 UTC (permalink / raw)
To: Johannes Schneider, barebox; +Cc: thomas.haemmerle, mgr
Hello,
On 6/2/26 6:09 AM, Johannes Schneider wrote:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> The i.MX8MP has a new LCDIF V8 display controller with a different
> register layout from the older mxsfb-based LCDIF found on i.MX6/7/8MM.
> The existing DRIVER_VIDEO_LCDIF driver does not support this variant,
> leaving the i.MX8MP without a barebox framebuffer and no way to show a
> boot splash.
>
> The driver:
> - Programs LCDIF V8 timing registers for the configured video mode.
> - Uses 128B DMA burst size (P_SIZE=1, T_SIZE=1) so that 800-pixel rows
> (3200 bytes) divide into exactly 25 complete bursts; 256B bursts leave
> a partial burst and produce a ~32px black strip at the right edge.
> - Sets correct CTRL polarity bit positions (INV_HS=bit0, INV_VS=bit1,
> INV_DE=bit2, INV_PXCK=bit3); the i.MX8MP TRM places these at bits 0-3,
> not 2-5 as a naive port from mxsfb would suggest.
> - Allocates a write-combine DMA framebuffer and registers a simplefb DT
> fixup for Linux DRM_SIMPLEDRM to inherit the boot image.
> - Enables the framebuffer at probe time so the splash command only needs
> to blit pixels; fb_enable() is not called per-splash.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
Michael had introduced this driver already into barebox v2026.01.0, see
drivers/video/lcdif_drv.c.
It matches both i.MX8MP and i.MX93 already, but has been tested only on
i.MX93 so far as I am aware.
Please try again with the upstream driver and extend it as needed.
Cheers,
Ahmad
> ---
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/imx-lcdif.c | 378 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 389 insertions(+)
> create mode 100644 drivers/video/imx-lcdif.c
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index ce10237221..fbcec6fd67 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -194,6 +194,16 @@ config DRIVER_VIDEO_TC358767
> help
> The TC358767A is a DSI/DPI to eDP video encoder chip
>
> +config DRIVER_VIDEO_IMX_LCDIF
> + bool "i.MX8MP LCDIF V8 framebuffer driver"
> + select VIDEO_VPL
> + select DRIVER_VIDEO_SIMPLEFB
> + depends on OFTREE && OFDEVICE
> + help
> + Framebuffer driver for the i.MX8MP/i.MX93 LCDIF V8 display controller.
> + Supports the "fsl,imx8mp-lcdif" compatible. Creates a simple-framebuffer
> + device tree node for kernel handoff via DRM_SIMPLEDRM.
> +
> config DRIVER_VIDEO_SIMPLE_PANEL
> bool "Simple panel support"
> select VIDEO_VPL
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 7b10eda0d8..9957ff5ad2 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbconsole.o
> obj-$(CONFIG_VIDEO_VPL) += vpl.o
> obj-$(CONFIG_DRIVER_VIDEO_MTL017) += mtl017.o
> obj-$(CONFIG_DRIVER_VIDEO_TC358767) += tc358767.o
> +obj-$(CONFIG_DRIVER_VIDEO_IMX_LCDIF) += imx-lcdif.o
> obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_PANEL) += simple-panel.o
> obj-$(CONFIG_DRIVER_VIDEO_MIPI_DBI) += mipi_dbi.o
> obj-$(CONFIG_DRIVER_VIDEO_MIPI_DSI) += mipi_dsi.o
> diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> new file mode 100644
> index 0000000000..ae5976c771
> --- /dev/null
> +++ b/drivers/video/imx-lcdif.c
> @@ -0,0 +1,378 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * i.MX8MP LCDIF V8 framebuffer driver for barebox
> + *
> + * Based on Linux drivers/gpu/drm/mxsfb/lcdif_drv.c and lcdif_kms.c
> + * Copyright (C) 2022 Marek Vasut <marex@denx.de>
> + *
> + * Copyright Leica Geosystems AG
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <driver.h>
> +#include <io.h>
> +#include <dma.h>
> +#include <fb.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <of_graph.h>
> +#include <video/media-bus-format.h>
> +#include <video/vpl.h>
> +
> +/* LCDIF V8 register map */
> +#define LCDC_V8_CTRL 0x00
> +#define LCDC_V8_DISP_PARA 0x10
> +#define LCDC_V8_DISP_SIZE 0x14
> +#define LCDC_V8_HSYN_PARA 0x18
> +#define LCDC_V8_VSYN_PARA 0x1c
> +#define LCDC_V8_VSYN_HSYN_WIDTH 0x20
> +#define LCDC_V8_INT_STATUS_D0 0x24
> +#define LCDC_V8_INT_ENABLE_D0 0x28
> +#define LCDC_V8_INT_STATUS_D1 0x30
> +#define LCDC_V8_INT_ENABLE_D1 0x34
> +#define LCDC_V8_CTRLDESCL0_1 0x200
> +#define LCDC_V8_CTRLDESCL0_3 0x208
> +#define LCDC_V8_CTRLDESCL_LOW0_4 0x20c
> +#define LCDC_V8_CTRLDESCL_HIGH0_4 0x210
> +#define LCDC_V8_CTRLDESCL0_5 0x214
> +#define LCDC_V8_CSC0_CTRL 0x21c
> +#define LCDC_V8_PANIC0_THRES 0x238
> +
> +/* CTRL bits */
> +#define CTRL_SW_RESET BIT(31)
> +#define CTRL_FETCH_START_OPTION_FPV 0
> +#define CTRL_INV_HS BIT(0)
> +#define CTRL_INV_VS BIT(1)
> +#define CTRL_INV_DE BIT(2)
> +#define CTRL_INV_PXCK BIT(3)
> +
> +/* DISP_PARA bits */
> +#define DISP_PARA_DISP_ON BIT(31)
> +#define DISP_PARA_LINE_PATTERN_RGB888 (0x0 << 26)
> +#define DISP_PARA_LINE_PATTERN_UYVY (0x3 << 26)
> +#define DISP_PARA_LINE_PATTERN_MASK GENMASK(29, 26)
> +
> +/* DISP_SIZE: Y[31:16] X[15:0] */
> +#define DISP_SIZE_DELTA_Y(y) (((y) & 0xffff) << 16)
> +#define DISP_SIZE_DELTA_X(x) ((x) & 0xffff)
> +
> +/* HSYN_PARA: back_porch[31:16] front_porch[15:0] */
> +#define HSYN_PARA_BP_H(bp) (((bp) & 0xffff) << 16)
> +#define HSYN_PARA_FP_H(fp) ((fp) & 0xffff)
> +
> +/* VSYN_PARA: back_porch[31:16] front_porch[15:0] */
> +#define VSYN_PARA_BP_V(bp) (((bp) & 0xffff) << 16)
> +#define VSYN_PARA_FP_V(fp) ((fp) & 0xffff)
> +
> +/* VSYN_HSYN_WIDTH: vsync[31:16] hsync[15:0] */
> +#define VSYN_HSYN_WIDTH_PW_V(v) (((v) & 0xffff) << 16)
> +#define VSYN_HSYN_WIDTH_PW_H(h) ((h) & 0xffff)
> +
> +/* CTRLDESCL0_1: height[31:16] width[15:0] */
> +#define CTRLDESCL0_1_HEIGHT(h) (((h) & 0xffff) << 16)
> +#define CTRLDESCL0_1_WIDTH(w) ((w) & 0xffff)
> +
> +/* CTRLDESCL0_3: P_SIZE[22:20] T_SIZE[17:16] PITCH[15:0] */
> +#define CTRLDESCL0_3_P_SIZE(s) (((s) & 0x7) << 20)
> +#define CTRLDESCL0_3_T_SIZE(s) (((s) & 0x3) << 16)
> +#define CTRLDESCL0_3_PITCH(p) ((p) & 0xffff)
> +
> +/* CTRLDESCL0_5 */
> +#define CTRLDESCL0_5_EN BIT(31)
> +#define CTRLDESCL0_5_SHADOW_LOAD_EN BIT(30)
> +#define CTRLDESCL0_5_BPP_16_RGB565 (0x4 << 24)
> +#define CTRLDESCL0_5_BPP_24_RGB888 (0x8 << 24)
> +#define CTRLDESCL0_5_BPP_32_ARGB8888 (0x9 << 24)
> +#define CTRLDESCL0_5_BPP_MASK GENMASK(27, 24)
> +
> +/* CSC0_CTRL */
> +#define CSC0_CTRL_BYPASS BIT(0)
> +
> +/* INT_ENABLE_D1 */
> +#define INT_ENABLE_D1_PLANE_PANIC_EN BIT(0)
> +
> +/* PANIC0_THRES */
> +#define PANIC0_THRES_LOW_MASK GENMASK(8, 0)
> +#define PANIC0_THRES_HIGH_MASK GENMASK(24, 16)
> +#define PANIC0_THRES_MAX 511
> +
> +struct lcdif_priv {
> + void __iomem *base;
> + struct clk *clk_pix;
> + struct clk *clk_axi;
> + struct clk *clk_disp_axi;
> + struct fb_info info;
> + struct vpl vpl;
> + int port_id;
> +};
> +
> +static void lcdif_reset(struct lcdif_priv *priv)
> +{
> + writel(CTRL_SW_RESET, priv->base + LCDC_V8_CTRL);
> + udelay(10);
> + writel(0, priv->base + LCDC_V8_CTRL);
> +}
> +
> +static void lcdif_set_mode(struct lcdif_priv *priv, struct fb_videomode *mode)
> +{
> + u32 ctrl = 0;
> +
> + if (mode->sync & FB_SYNC_HOR_HIGH_ACT)
> + ; /* HSYNC active high = no invert */
> + else
> + ctrl |= CTRL_INV_HS;
> +
> + if (mode->sync & FB_SYNC_VERT_HIGH_ACT)
> + ; /* VSYNC active high = no invert */
> + else
> + ctrl |= CTRL_INV_VS;
> +
> + if (mode->display_flags & DISPLAY_FLAGS_DE_LOW)
> + ctrl |= CTRL_INV_DE;
> + if (mode->display_flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + ctrl |= CTRL_INV_PXCK;
> +
> + writel(ctrl, priv->base + LCDC_V8_CTRL);
> +
> + writel(DISP_SIZE_DELTA_Y(mode->yres) | DISP_SIZE_DELTA_X(mode->xres),
> + priv->base + LCDC_V8_DISP_SIZE);
> +
> + /* hback_porch = htotal - hsync_end, hfront_porch = hsync_start - hactive */
> + writel(HSYN_PARA_BP_H(mode->left_margin) | HSYN_PARA_FP_H(mode->right_margin),
> + priv->base + LCDC_V8_HSYN_PARA);
> +
> + writel(VSYN_PARA_BP_V(mode->upper_margin) | VSYN_PARA_FP_V(mode->lower_margin),
> + priv->base + LCDC_V8_VSYN_PARA);
> +
> + writel(VSYN_HSYN_WIDTH_PW_V(mode->vsync_len) | VSYN_HSYN_WIDTH_PW_H(mode->hsync_len),
> + priv->base + LCDC_V8_VSYN_HSYN_WIDTH);
> +
> + writel(CTRLDESCL0_1_HEIGHT(mode->yres) | CTRLDESCL0_1_WIDTH(mode->xres),
> + priv->base + LCDC_V8_CTRLDESCL0_1);
> +}
> +
> +static void lcdif_set_plane(struct lcdif_priv *priv)
> +{
> + struct fb_info *info = &priv->info;
> + phys_addr_t paddr = virt_to_phys(info->screen_base);
> + u32 ctrl;
> + u32 stride = info->line_length;
> +
> + /*
> + * AXI burst size: P_SIZE=1 = 128B, T_SIZE=1 = 128B threshold.
> + * Use 128B (32-pixel @ 32bpp) bursts so that 800-pixel rows
> + * (3200 bytes) divide evenly: 3200 / 128 = 25 complete bursts.
> + * P_SIZE=2 (256B) would leave a partial burst (3200/256 = 12.5)
> + * causing a ~32-pixel dark gap at the right edge of the display.
> + */
> + ctrl = CTRLDESCL0_3_P_SIZE(1) | CTRLDESCL0_3_T_SIZE(1) |
> + CTRLDESCL0_3_PITCH(stride);
> + writel(ctrl, priv->base + LCDC_V8_CTRLDESCL0_3);
> +
> + writel(lower_32_bits(paddr), priv->base + LCDC_V8_CTRLDESCL_LOW0_4);
> + writel(upper_32_bits(paddr), priv->base + LCDC_V8_CTRLDESCL_HIGH0_4);
> +
> + /* 32bpp XRGB8888 → ARGB8888 pixel format.
> + * SHADOW_LOAD_EN causes the descriptor registers (address, format, pitch)
> + * to be latched into the active registers on the next VSYNC, which is
> + * required for the initial descriptor load in barebox's single-enable
> + * sequence (no DRM page-flip mechanism). EN is ORed in later by
> + * lcdif_enable() after DISP_ON. */
> + writel(CTRLDESCL0_5_BPP_32_ARGB8888 | CTRLDESCL0_5_SHADOW_LOAD_EN,
> + priv->base + LCDC_V8_CTRLDESCL0_5);
> +}
> +
> +static void lcdif_enable(struct lcdif_priv *priv)
> +{
> + u32 reg;
> +
> + /* Set FIFO Panic watermarks: low=1/3, high=2/3 */
> + writel(FIELD_PREP(PANIC0_THRES_LOW_MASK, 1 * PANIC0_THRES_MAX / 3) |
> + FIELD_PREP(PANIC0_THRES_HIGH_MASK, 2 * PANIC0_THRES_MAX / 3),
> + priv->base + LCDC_V8_PANIC0_THRES);
> +
> + writel(INT_ENABLE_D1_PLANE_PANIC_EN, priv->base + LCDC_V8_INT_ENABLE_D1);
> +
> + /* Write LINE_PATTERN_RGB888 (= 0) first to clear all DISP_PARA bits,
> + * including BGND_R/G/B (bits [23:0]) — ensures background is black. */
> + writel(DISP_PARA_LINE_PATTERN_RGB888, priv->base + LCDC_V8_DISP_PARA);
> + reg = readl(priv->base + LCDC_V8_DISP_PARA);
> + reg |= DISP_PARA_DISP_ON;
> + writel(reg, priv->base + LCDC_V8_DISP_PARA);
> +
> + reg = readl(priv->base + LCDC_V8_CTRLDESCL0_5);
> + reg |= CTRLDESCL0_5_EN;
> + writel(reg, priv->base + LCDC_V8_CTRLDESCL0_5);
> +}
> +
> +static int lcdif_fb_enable(struct fb_info *info)
> +{
> + struct lcdif_priv *priv = info->priv;
> + struct fb_videomode *mode = info->mode;
> + int ret;
> +
> + clk_set_rate(priv->clk_pix, PICOS2KHZ(mode->pixclock) * 1000UL);
> + clk_prepare_enable(priv->clk_axi);
> + clk_prepare_enable(priv->clk_disp_axi);
> + clk_prepare_enable(priv->clk_pix);
> +
> + /* Notify downstream bridge about mode and enable it */
> + ret = vpl_ioctl_prepare(&priv->vpl, priv->port_id, mode);
> + if (ret) {
> + dev_err(info->dev.parent, "vpl_prepare failed: %pe\n", ERR_PTR(ret));
> + return ret;
> + }
> + lcdif_reset(priv);
> + lcdif_set_mode(priv, mode);
> + lcdif_set_plane(priv);
> +
> + /* Bypass CSC for RGB input */
> + writel(CSC0_CTRL_BYPASS, priv->base + LCDC_V8_CSC0_CTRL);
> +
> + lcdif_enable(priv);
> +
> + ret = vpl_ioctl_enable(&priv->vpl, priv->port_id);
> + if (ret)
> + dev_warn(info->dev.parent, "vpl_enable failed: %pe\n", ERR_PTR(ret));
> +
> + return 0;
> +}
> +
> +static void lcdif_fb_disable(struct fb_info *info)
> +{
> + struct lcdif_priv *priv = info->priv;
> + u32 reg;
> +
> + vpl_ioctl_disable(&priv->vpl, priv->port_id);
> +
> + reg = readl(priv->base + LCDC_V8_CTRLDESCL0_5);
> + reg &= ~CTRLDESCL0_5_EN;
> + writel(reg, priv->base + LCDC_V8_CTRLDESCL0_5);
> +
> + reg = readl(priv->base + LCDC_V8_DISP_PARA);
> + reg &= ~DISP_PARA_DISP_ON;
> + writel(reg, priv->base + LCDC_V8_DISP_PARA);
> +
> + vpl_ioctl_unprepare(&priv->vpl, priv->port_id);
> +
> + clk_disable_unprepare(priv->clk_pix);
> + clk_disable_unprepare(priv->clk_disp_axi);
> + clk_disable_unprepare(priv->clk_axi);
> +}
> +
> +static struct fb_ops lcdif_fb_ops = {
> + .fb_enable = lcdif_fb_enable,
> + .fb_disable = lcdif_fb_disable,
> +};
> +
> +static int lcdif_probe(struct device *dev)
> +{
> + struct resource *iores;
> + struct lcdif_priv *priv;
> + struct fb_info *info;
> + int ret;
> +
> + iores = dev_request_mem_resource(dev, 0);
> + if (IS_ERR(iores))
> + return PTR_ERR(iores);
> +
> + priv = xzalloc(sizeof(*priv));
> + priv->base = IOMEM(iores->start);
> +
> + priv->clk_pix = clk_get(dev, "pix");
> + if (IS_ERR(priv->clk_pix))
> + return dev_errp_probe(dev, priv->clk_pix, "pixel clock\n");
> +
> + priv->clk_axi = clk_get(dev, "axi");
> + if (IS_ERR(priv->clk_axi))
> + return dev_errp_probe(dev, priv->clk_axi, "axi clock\n");
> +
> + priv->clk_disp_axi = clk_get(dev, "disp_axi");
> + if (IS_ERR(priv->clk_disp_axi))
> + return dev_errp_probe(dev, priv->clk_disp_axi, "disp_axi clock\n");
> +
> + /* Register VPL node - endpoint is port@0 */
> + priv->port_id = 0;
> + priv->vpl.node = dev->of_node;
> + ret = vpl_register(&priv->vpl);
> + if (ret)
> + return ret;
> +
> + info = &priv->info;
> + info->priv = priv;
> + info->fbops = &lcdif_fb_ops;
> +
> + /* Fixed 32bpp XRGB8888 format */
> + info->bits_per_pixel = 32;
> + info->red = (struct fb_bitfield){ .offset = 16, .length = 8 };
> + info->green = (struct fb_bitfield){ .offset = 8, .length = 8 };
> + info->blue = (struct fb_bitfield){ .offset = 0, .length = 8 };
> + info->transp = (struct fb_bitfield){ .offset = 24, .length = 8 };
> +
> + /* Get display modes from downstream panel via VPL */
> + ret = vpl_ioctl(&priv->vpl, priv->port_id, VPL_GET_VIDEOMODES, &info->modes);
> + if (ret) {
> + dev_dbg(dev, "no modes from VPL: %pe\n", ERR_PTR(ret));
> + return ret;
> + }
> +
> + /* Allocate DMA-coherent framebuffer */
> + if (info->modes.num_modes > 0) {
> + struct fb_videomode *m = &info->modes.modes[0];
> + size_t fb_size = m->xres * m->yres * (info->bits_per_pixel / 8);
> +
> + info->screen_base = dma_alloc_writecombine(DMA_DEVICE_BROKEN,
> + fb_size,
> + DMA_ADDRESS_BROKEN);
> + if (!info->screen_base) {
> + dev_err(dev, "failed to allocate framebuffer\n");
> + return -ENOMEM;
> + }
> + info->screen_size = fb_size;
> + memset(info->screen_base, 0, fb_size);
> + }
> +
> + info->dev.parent = dev;
> + ret = register_framebuffer(info);
> + if (ret) {
> + dev_err(dev, "failed to register framebuffer: %d\n", ret);
> + return ret;
> + }
> +
> + /*
> + * Create simple-framebuffer DT node for Linux kernel handoff.
> + * With CONFIG_DRM_SIMPLEDRM, Linux will inherit the barebox
> + * framebuffer and keep the boot splash visible until platsch
> + * renders via the LCDIF DRM driver.
> + */
> + info->register_simplefb = 1;
> + fb_register_simplefb(info);
> +
> + /*
> + * Enable the display immediately so the framebuffer is live.
> + * The splash command only blits pixels to the buffer - it does not
> + * call fb_enable itself - so we must enable here during probe.
> + */
> + ret = fb_enable(info);
> + if (ret)
> + dev_warn(dev, "failed to enable framebuffer: %d\n", ret);
> +
> + dev_info(dev, "i.MX8MP LCDIF registered\n");
> + return 0;
> +}
> +
> +static const struct of_device_id lcdif_dt_ids[] = {
> + { .compatible = "fsl,imx8mp-lcdif" },
> + { .compatible = "fsl,imx93-lcdif" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, lcdif_dt_ids);
> +
> +static struct driver lcdif_driver = {
> + .name = "imx-lcdif",
> + .probe = lcdif_probe,
> + .of_compatible = DRV_OF_COMPAT(lcdif_dt_ids),
> +};
> +device_platform_driver(lcdif_driver);
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/6] video: add i.MX8MP LCDIF2 V8 framebuffer driver
2026-06-02 7:16 ` Ahmad Fatoum
@ 2026-06-04 3:34 ` SCHNEIDER Johannes
0 siblings, 0 replies; 19+ messages in thread
From: SCHNEIDER Johannes @ 2026-06-04 3:34 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: HAEMMERLE Thomas, mgr, barebox
Hoi Ahmad,
>
> Hello,
>
> On 6/2/26 6:09 AM, Johannes Schneider wrote:
> > From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> >
> > The i.MX8MP has a new LCDIF V8 display controller with a different
> > register layout from the older mxsfb-based LCDIF found on i.MX6/7/8MM.
> > The existing DRIVER_VIDEO_LCDIF driver does not support this variant,
> > leaving the i.MX8MP without a barebox framebuffer and no way to show a
> > boot splash.
> >
> > The driver:
> > - Programs LCDIF V8 timing registers for the configured video mode.
> > - Uses 128B DMA burst size (P_SIZE=1, T_SIZE=1) so that 800-pixel rows
> > (3200 bytes) divide into exactly 25 complete bursts; 256B bursts leave
> > a partial burst and produce a ~32px black strip at the right edge.
> > - Sets correct CTRL polarity bit positions (INV_HS=bit0, INV_VS=bit1,
> > INV_DE=bit2, INV_PXCK=bit3); the i.MX8MP TRM places these at bits 0-3,
> > not 2-5 as a naive port from mxsfb would suggest.
> > - Allocates a write-combine DMA framebuffer and registers a simplefb DT
> > fixup for Linux DRM_SIMPLEDRM to inherit the boot image.
> > - Enables the framebuffer at probe time so the splash command only needs
> > to blit pixels; fb_enable() is not called per-splash.
> >
> > Assisted-by: Claude:claude-sonnet-4-6
> > Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> Michael had introduced this driver already into barebox v2026.01.0, see
> drivers/video/lcdif_drv.c.
>
ups, that one we totally overlooked :-|
> It matches both i.MX8MP and i.MX93 already, but has been tested only on
> i.MX93 so far as I am aware.
>
> Please try again with the upstream driver and extend it as needed.
>
will do, i'll restructure the other commits accordingly, test it on our 8mp
device(s) and send out a v2 of the whole patchstack.
gruß
Johannes
> Cheers,
> Ahmad
>
> > ---
> > drivers/video/Kconfig | 10 +
> > drivers/video/Makefile | 1 +
> > drivers/video/imx-lcdif.c | 378 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 389 insertions(+)
> > create mode 100644 drivers/video/imx-lcdif.c
> >
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index ce10237221..fbcec6fd67 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -194,6 +194,16 @@ config DRIVER_VIDEO_TC358767
> > help
> > The TC358767A is a DSI/DPI to eDP video encoder chip
> >
> > +config DRIVER_VIDEO_IMX_LCDIF
> > + bool "i.MX8MP LCDIF V8 framebuffer driver"
> > + select VIDEO_VPL
> > + select DRIVER_VIDEO_SIMPLEFB
> > + depends on OFTREE && OFDEVICE
> > + help
> > + Framebuffer driver for the i.MX8MP/i.MX93 LCDIF V8 display controller.
> > + Supports the "fsl,imx8mp-lcdif" compatible. Creates a simple-framebuffer
> > + device tree node for kernel handoff via DRM_SIMPLEDRM.
> > +
> > config DRIVER_VIDEO_SIMPLE_PANEL
> > bool "Simple panel support"
> > select VIDEO_VPL
> > diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> > index 7b10eda0d8..9957ff5ad2 100644
> > --- a/drivers/video/Makefile
> > +++ b/drivers/video/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbconsole.o
> > obj-$(CONFIG_VIDEO_VPL) += vpl.o
> > obj-$(CONFIG_DRIVER_VIDEO_MTL017) += mtl017.o
> > obj-$(CONFIG_DRIVER_VIDEO_TC358767) += tc358767.o
> > +obj-$(CONFIG_DRIVER_VIDEO_IMX_LCDIF) += imx-lcdif.o
> > obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_PANEL) += simple-panel.o
> > obj-$(CONFIG_DRIVER_VIDEO_MIPI_DBI) += mipi_dbi.o
> > obj-$(CONFIG_DRIVER_VIDEO_MIPI_DSI) += mipi_dsi.o
> > diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> > new file mode 100644
> > index 0000000000..ae5976c771
> > --- /dev/null
> > +++ b/drivers/video/imx-lcdif.c
> > @@ -0,0 +1,378 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * i.MX8MP LCDIF V8 framebuffer driver for barebox
> > + *
> > + * Based on Linux drivers/gpu/drm/mxsfb/lcdif_drv.c and lcdif_kms.c
> > + * Copyright (C) 2022 Marek Vasut <marex@denx.de>
> > + *
> > + * Copyright Leica Geosystems AG
> > + */
> > +
> > +#include <common.h>
> > +#include <init.h>
> > +#include <driver.h>
> > +#include <io.h>
> > +#include <dma.h>
> > +#include <fb.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <of_graph.h>
> > +#include <video/media-bus-format.h>
> > +#include <video/vpl.h>
> > +
> > +/* LCDIF V8 register map */
> > +#define LCDC_V8_CTRL 0x00
> > +#define LCDC_V8_DISP_PARA 0x10
> > +#define LCDC_V8_DISP_SIZE 0x14
> > +#define LCDC_V8_HSYN_PARA 0x18
> > +#define LCDC_V8_VSYN_PARA 0x1c
> > +#define LCDC_V8_VSYN_HSYN_WIDTH 0x20
> > +#define LCDC_V8_INT_STATUS_D0 0x24
> > +#define LCDC_V8_INT_ENABLE_D0 0x28
> > +#define LCDC_V8_INT_STATUS_D1 0x30
> > +#define LCDC_V8_INT_ENABLE_D1 0x34
> > +#define LCDC_V8_CTRLDESCL0_1 0x200
> > +#define LCDC_V8_CTRLDESCL0_3 0x208
> > +#define LCDC_V8_CTRLDESCL_LOW0_4 0x20c
> > +#define LCDC_V8_CTRLDESCL_HIGH0_4 0x210
> > +#define LCDC_V8_CTRLDESCL0_5 0x214
> > +#define LCDC_V8_CSC0_CTRL 0x21c
> > +#define LCDC_V8_PANIC0_THRES 0x238
> > +
> > +/* CTRL bits */
> > +#define CTRL_SW_RESET BIT(31)
> > +#define CTRL_FETCH_START_OPTION_FPV 0
> > +#define CTRL_INV_HS BIT(0)
> > +#define CTRL_INV_VS BIT(1)
> > +#define CTRL_INV_DE BIT(2)
> > +#define CTRL_INV_PXCK BIT(3)
> > +
> > +/* DISP_PARA bits */
> > +#define DISP_PARA_DISP_ON BIT(31)
> > +#define DISP_PARA_LINE_PATTERN_RGB888 (0x0 << 26)
> > +#define DISP_PARA_LINE_PATTERN_UYVY (0x3 << 26)
> > +#define DISP_PARA_LINE_PATTERN_MASK GENMASK(29, 26)
> > +
> > +/* DISP_SIZE: Y[31:16] X[15:0] */
> > +#define DISP_SIZE_DELTA_Y(y) (((y) & 0xffff) << 16)
> > +#define DISP_SIZE_DELTA_X(x) ((x) & 0xffff)
> > +
> > +/* HSYN_PARA: back_porch[31:16] front_porch[15:0] */
> > +#define HSYN_PARA_BP_H(bp) (((bp) & 0xffff) << 16)
> > +#define HSYN_PARA_FP_H(fp) ((fp) & 0xffff)
> > +
> > +/* VSYN_PARA: back_porch[31:16] front_porch[15:0] */
> > +#define VSYN_PARA_BP_V(bp) (((bp) & 0xffff) << 16)
> > +#define VSYN_PARA_FP_V(fp) ((fp) & 0xffff)
> > +
> > +/* VSYN_HSYN_WIDTH: vsync[31:16] hsync[15:0] */
> > +#define VSYN_HSYN_WIDTH_PW_V(v) (((v) & 0xffff) << 16)
> > +#define VSYN_HSYN_WIDTH_PW_H(h) ((h) & 0xffff)
> > +
> > +/* CTRLDESCL0_1: height[31:16] width[15:0] */
> > +#define CTRLDESCL0_1_HEIGHT(h) (((h) & 0xffff) << 16)
> > +#define CTRLDESCL0_1_WIDTH(w) ((w) & 0xffff)
> > +
> > +/* CTRLDESCL0_3: P_SIZE[22:20] T_SIZE[17:16] PITCH[15:0] */
> > +#define CTRLDESCL0_3_P_SIZE(s) (((s) & 0x7) << 20)
> > +#define CTRLDESCL0_3_T_SIZE(s) (((s) & 0x3) << 16)
> > +#define CTRLDESCL0_3_PITCH(p) ((p) & 0xffff)
> > +
> > +/* CTRLDESCL0_5 */
> > +#define CTRLDESCL0_5_EN BIT(31)
> > +#define CTRLDESCL0_5_SHADOW_LOAD_EN BIT(30)
> > +#define CTRLDESCL0_5_BPP_16_RGB565 (0x4 << 24)
> > +#define CTRLDESCL0_5_BPP_24_RGB888 (0x8 << 24)
> > +#define CTRLDESCL0_5_BPP_32_ARGB8888 (0x9 << 24)
> > +#define CTRLDESCL0_5_BPP_MASK GENMASK(27, 24)
> > +
> > +/* CSC0_CTRL */
> > +#define CSC0_CTRL_BYPASS BIT(0)
> > +
> > +/* INT_ENABLE_D1 */
> > +#define INT_ENABLE_D1_PLANE_PANIC_EN BIT(0)
> > +
> > +/* PANIC0_THRES */
> > +#define PANIC0_THRES_LOW_MASK GENMASK(8, 0)
> > +#define PANIC0_THRES_HIGH_MASK GENMASK(24, 16)
> > +#define PANIC0_THRES_MAX 511
> > +
> > +struct lcdif_priv {
> > + void __iomem *base;
> > + struct clk *clk_pix;
> > + struct clk *clk_axi;
> > + struct clk *clk_disp_axi;
> > + struct fb_info info;
> > + struct vpl vpl;
> > + int port_id;
> > +};
> > +
> > +static void lcdif_reset(struct lcdif_priv *priv)
> > +{
> > + writel(CTRL_SW_RESET, priv->base + LCDC_V8_CTRL);
> > + udelay(10);
> > + writel(0, priv->base + LCDC_V8_CTRL);
> > +}
> > +
> > +static void lcdif_set_mode(struct lcdif_priv *priv, struct fb_videomode *mode)
> > +{
> > + u32 ctrl = 0;
> > +
> > + if (mode->sync & FB_SYNC_HOR_HIGH_ACT)
> > + ; /* HSYNC active high = no invert */
> > + else
> > + ctrl |= CTRL_INV_HS;
> > +
> > + if (mode->sync & FB_SYNC_VERT_HIGH_ACT)
> > + ; /* VSYNC active high = no invert */
> > + else
> > + ctrl |= CTRL_INV_VS;
> > +
> > + if (mode->display_flags & DISPLAY_FLAGS_DE_LOW)
> > + ctrl |= CTRL_INV_DE;
> > + if (mode->display_flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > + ctrl |= CTRL_INV_PXCK;
> > +
> > + writel(ctrl, priv->base + LCDC_V8_CTRL);
> > +
> > + writel(DISP_SIZE_DELTA_Y(mode->yres) | DISP_SIZE_DELTA_X(mode->xres),
> > + priv->base + LCDC_V8_DISP_SIZE);
> > +
> > + /* hback_porch = htotal - hsync_end, hfront_porch = hsync_start - hactive */
> > + writel(HSYN_PARA_BP_H(mode->left_margin) | HSYN_PARA_FP_H(mode->right_margin),
> > + priv->base + LCDC_V8_HSYN_PARA);
> > +
> > + writel(VSYN_PARA_BP_V(mode->upper_margin) | VSYN_PARA_FP_V(mode->lower_margin),
> > + priv->base + LCDC_V8_VSYN_PARA);
> > +
> > + writel(VSYN_HSYN_WIDTH_PW_V(mode->vsync_len) | VSYN_HSYN_WIDTH_PW_H(mode->hsync_len),
> > + priv->base + LCDC_V8_VSYN_HSYN_WIDTH);
> > +
> > + writel(CTRLDESCL0_1_HEIGHT(mode->yres) | CTRLDESCL0_1_WIDTH(mode->xres),
> > + priv->base + LCDC_V8_CTRLDESCL0_1);
> > +}
> > +
> > +static void lcdif_set_plane(struct lcdif_priv *priv)
> > +{
> > + struct fb_info *info = &priv->info;
> > + phys_addr_t paddr = virt_to_phys(info->screen_base);
> > + u32 ctrl;
> > + u32 stride = info->line_length;
> > +
> > + /*
> > + * AXI burst size: P_SIZE=1 = 128B, T_SIZE=1 = 128B threshold.
> > + * Use 128B (32-pixel @ 32bpp) bursts so that 800-pixel rows
> > + * (3200 bytes) divide evenly: 3200 / 128 = 25 complete bursts.
> > + * P_SIZE=2 (256B) would leave a partial burst (3200/256 = 12.5)
> > + * causing a ~32-pixel dark gap at the right edge of the display.
> > + */
> > + ctrl = CTRLDESCL0_3_P_SIZE(1) | CTRLDESCL0_3_T_SIZE(1) |
> > + CTRLDESCL0_3_PITCH(stride);
> > + writel(ctrl, priv->base + LCDC_V8_CTRLDESCL0_3);
> > +
> > + writel(lower_32_bits(paddr), priv->base + LCDC_V8_CTRLDESCL_LOW0_4);
> > + writel(upper_32_bits(paddr), priv->base + LCDC_V8_CTRLDESCL_HIGH0_4);
> > +
> > + /* 32bpp XRGB8888 → ARGB8888 pixel format.
> > + * SHADOW_LOAD_EN causes the descriptor registers (address, format, pitch)
> > + * to be latched into the active registers on the next VSYNC, which is
> > + * required for the initial descriptor load in barebox's single-enable
> > + * sequence (no DRM page-flip mechanism). EN is ORed in later by
> > + * lcdif_enable() after DISP_ON. */
> > + writel(CTRLDESCL0_5_BPP_32_ARGB8888 | CTRLDESCL0_5_SHADOW_LOAD_EN,
> > + priv->base + LCDC_V8_CTRLDESCL0_5);
> > +}
> > +
> > +static void lcdif_enable(struct lcdif_priv *priv)
> > +{
> > + u32 reg;
> > +
> > + /* Set FIFO Panic watermarks: low=1/3, high=2/3 */
> > + writel(FIELD_PREP(PANIC0_THRES_LOW_MASK, 1 * PANIC0_THRES_MAX / 3) |
> > + FIELD_PREP(PANIC0_THRES_HIGH_MASK, 2 * PANIC0_THRES_MAX / 3),
> > + priv->base + LCDC_V8_PANIC0_THRES);
> > +
> > + writel(INT_ENABLE_D1_PLANE_PANIC_EN, priv->base + LCDC_V8_INT_ENABLE_D1);
> > +
> > + /* Write LINE_PATTERN_RGB888 (= 0) first to clear all DISP_PARA bits,
> > + * including BGND_R/G/B (bits [23:0]) — ensures background is black. */
> > + writel(DISP_PARA_LINE_PATTERN_RGB888, priv->base + LCDC_V8_DISP_PARA);
> > + reg = readl(priv->base + LCDC_V8_DISP_PARA);
> > + reg |= DISP_PARA_DISP_ON;
> > + writel(reg, priv->base + LCDC_V8_DISP_PARA);
> > +
> > + reg = readl(priv->base + LCDC_V8_CTRLDESCL0_5);
> > + reg |= CTRLDESCL0_5_EN;
> > + writel(reg, priv->base + LCDC_V8_CTRLDESCL0_5);
> > +}
> > +
> > +static int lcdif_fb_enable(struct fb_info *info)
> > +{
> > + struct lcdif_priv *priv = info->priv;
> > + struct fb_videomode *mode = info->mode;
> > + int ret;
> > +
> > + clk_set_rate(priv->clk_pix, PICOS2KHZ(mode->pixclock) * 1000UL);
> > + clk_prepare_enable(priv->clk_axi);
> > + clk_prepare_enable(priv->clk_disp_axi);
> > + clk_prepare_enable(priv->clk_pix);
> > +
> > + /* Notify downstream bridge about mode and enable it */
> > + ret = vpl_ioctl_prepare(&priv->vpl, priv->port_id, mode);
> > + if (ret) {
> > + dev_err(info->dev.parent, "vpl_prepare failed: %pe\n", ERR_PTR(ret));
> > + return ret;
> > + }
> > + lcdif_reset(priv);
> > + lcdif_set_mode(priv, mode);
> > + lcdif_set_plane(priv);
> > +
> > + /* Bypass CSC for RGB input */
> > + writel(CSC0_CTRL_BYPASS, priv->base + LCDC_V8_CSC0_CTRL);
> > +
> > + lcdif_enable(priv);
> > +
> > + ret = vpl_ioctl_enable(&priv->vpl, priv->port_id);
> > + if (ret)
> > + dev_warn(info->dev.parent, "vpl_enable failed: %pe\n", ERR_PTR(ret));
> > +
> > + return 0;
> > +}
> > +
> > +static void lcdif_fb_disable(struct fb_info *info)
> > +{
> > + struct lcdif_priv *priv = info->priv;
> > + u32 reg;
> > +
> > + vpl_ioctl_disable(&priv->vpl, priv->port_id);
> > +
> > + reg = readl(priv->base + LCDC_V8_CTRLDESCL0_5);
> > + reg &= ~CTRLDESCL0_5_EN;
> > + writel(reg, priv->base + LCDC_V8_CTRLDESCL0_5);
> > +
> > + reg = readl(priv->base + LCDC_V8_DISP_PARA);
> > + reg &= ~DISP_PARA_DISP_ON;
> > + writel(reg, priv->base + LCDC_V8_DISP_PARA);
> > +
> > + vpl_ioctl_unprepare(&priv->vpl, priv->port_id);
> > +
> > + clk_disable_unprepare(priv->clk_pix);
> > + clk_disable_unprepare(priv->clk_disp_axi);
> > + clk_disable_unprepare(priv->clk_axi);
> > +}
> > +
> > +static struct fb_ops lcdif_fb_ops = {
> > + .fb_enable = lcdif_fb_enable,
> > + .fb_disable = lcdif_fb_disable,
> > +};
> > +
> > +static int lcdif_probe(struct device *dev)
> > +{
> > + struct resource *iores;
> > + struct lcdif_priv *priv;
> > + struct fb_info *info;
> > + int ret;
> > +
> > + iores = dev_request_mem_resource(dev, 0);
> > + if (IS_ERR(iores))
> > + return PTR_ERR(iores);
> > +
> > + priv = xzalloc(sizeof(*priv));
> > + priv->base = IOMEM(iores->start);
> > +
> > + priv->clk_pix = clk_get(dev, "pix");
> > + if (IS_ERR(priv->clk_pix))
> > + return dev_errp_probe(dev, priv->clk_pix, "pixel clock\n");
> > +
> > + priv->clk_axi = clk_get(dev, "axi");
> > + if (IS_ERR(priv->clk_axi))
> > + return dev_errp_probe(dev, priv->clk_axi, "axi clock\n");
> > +
> > + priv->clk_disp_axi = clk_get(dev, "disp_axi");
> > + if (IS_ERR(priv->clk_disp_axi))
> > + return dev_errp_probe(dev, priv->clk_disp_axi, "disp_axi clock\n");
> > +
> > + /* Register VPL node - endpoint is port@0 */
> > + priv->port_id = 0;
> > + priv->vpl.node = dev->of_node;
> > + ret = vpl_register(&priv->vpl);
> > + if (ret)
> > + return ret;
> > +
> > + info = &priv->info;
> > + info->priv = priv;
> > + info->fbops = &lcdif_fb_ops;
> > +
> > + /* Fixed 32bpp XRGB8888 format */
> > + info->bits_per_pixel = 32;
> > + info->red = (struct fb_bitfield){ .offset = 16, .length = 8 };
> > + info->green = (struct fb_bitfield){ .offset = 8, .length = 8 };
> > + info->blue = (struct fb_bitfield){ .offset = 0, .length = 8 };
> > + info->transp = (struct fb_bitfield){ .offset = 24, .length = 8 };
> > +
> > + /* Get display modes from downstream panel via VPL */
> > + ret = vpl_ioctl(&priv->vpl, priv->port_id, VPL_GET_VIDEOMODES, &info->modes);
> > + if (ret) {
> > + dev_dbg(dev, "no modes from VPL: %pe\n", ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + /* Allocate DMA-coherent framebuffer */
> > + if (info->modes.num_modes > 0) {
> > + struct fb_videomode *m = &info->modes.modes[0];
> > + size_t fb_size = m->xres * m->yres * (info->bits_per_pixel / 8);
> > +
> > + info->screen_base = dma_alloc_writecombine(DMA_DEVICE_BROKEN,
> > + fb_size,
> > + DMA_ADDRESS_BROKEN);
> > + if (!info->screen_base) {
> > + dev_err(dev, "failed to allocate framebuffer\n");
> > + return -ENOMEM;
> > + }
> > + info->screen_size = fb_size;
> > + memset(info->screen_base, 0, fb_size);
> > + }
> > +
> > + info->dev.parent = dev;
> > + ret = register_framebuffer(info);
> > + if (ret) {
> > + dev_err(dev, "failed to register framebuffer: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + /*
> > + * Create simple-framebuffer DT node for Linux kernel handoff.
> > + * With CONFIG_DRM_SIMPLEDRM, Linux will inherit the barebox
> > + * framebuffer and keep the boot splash visible until platsch
> > + * renders via the LCDIF DRM driver.
> > + */
> > + info->register_simplefb = 1;
> > + fb_register_simplefb(info);
> > +
> > + /*
> > + * Enable the display immediately so the framebuffer is live.
> > + * The splash command only blits pixels to the buffer - it does not
> > + * call fb_enable itself - so we must enable here during probe.
> > + */
> > + ret = fb_enable(info);
> > + if (ret)
> > + dev_warn(dev, "failed to enable framebuffer: %d\n", ret);
> > +
> > + dev_info(dev, "i.MX8MP LCDIF registered\n");
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id lcdif_dt_ids[] = {
> > + { .compatible = "fsl,imx8mp-lcdif" },
> > + { .compatible = "fsl,imx93-lcdif" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, lcdif_dt_ids);
> > +
> > +static struct driver lcdif_driver = {
> > + .name = "imx-lcdif",
> > + .probe = lcdif_probe,
> > + .of_compatible = DRV_OF_COMPAT(lcdif_dt_ids),
> > +};
> > +device_platform_driver(lcdif_driver);
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/6] video: add generic panel-lvds driver
2026-06-02 4:09 [PATCH 0/7] video: enable boot splash on i.MX8MP with LVDS panel Johannes Schneider
` (3 preceding siblings ...)
2026-06-02 4:09 ` [PATCH 4/6] video: add i.MX8MP LCDIF2 V8 framebuffer driver Johannes Schneider
@ 2026-06-02 4:09 ` Johannes Schneider
2026-06-02 7:12 ` Ahmad Fatoum
2026-06-02 4:09 ` [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels Johannes Schneider
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schneider @ 2026-06-02 4:09 UTC (permalink / raw)
To: barebox; +Cc: thomas.haemmerle
From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
Barebox has no driver for "panel-lvds" compatible DT nodes, which are used by
all i.MX8M board display pipelines to describe LVDS panels with a panel-timing
subnode. Without this driver the VPL chain in fsl-ldb terminates before it can
query video modes, and the LCDIF2 framebuffer driver cannot determine the active
display resolution, so no framebuffer is allocated and no splash is shown.
The driver reads one video mode from the panel-timing subnode, enables
an optional power-supply regulator and enable GPIO on VPL_ENABLE, and
controls an optional backlight device. Closely follows the Linux
drivers/gpu/drm/panel/panel-lvds.c behavior.
Assisted-by: Claude:claude-sonnet-4-6
Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
---
drivers/video/Kconfig | 9 ++
drivers/video/Makefile | 1 +
drivers/video/panel-lvds.c | 175 +++++++++++++++++++++++++++++++++++++
3 files changed, 185 insertions(+)
create mode 100644 drivers/video/panel-lvds.c
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index fbcec6fd67..964c2c8a29 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -204,6 +204,15 @@ config DRIVER_VIDEO_IMX_LCDIF
Supports the "fsl,imx8mp-lcdif" compatible. Creates a simple-framebuffer
device tree node for kernel handoff via DRM_SIMPLEDRM.
+config DRIVER_VIDEO_PANEL_LVDS
+ bool "Generic LVDS panel support"
+ select VIDEO_VPL
+ depends on OFTREE && OFDEVICE
+ help
+ Driver for "panel-lvds" compatible display panels. Reads the video
+ mode from a "panel-timing" subnode and controls the panel power
+ supply regulator.
+
config DRIVER_VIDEO_SIMPLE_PANEL
bool "Simple panel support"
select VIDEO_VPL
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9957ff5ad2..9d073fdf75 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_VIDEO_VPL) += vpl.o
obj-$(CONFIG_DRIVER_VIDEO_MTL017) += mtl017.o
obj-$(CONFIG_DRIVER_VIDEO_TC358767) += tc358767.o
obj-$(CONFIG_DRIVER_VIDEO_IMX_LCDIF) += imx-lcdif.o
+obj-$(CONFIG_DRIVER_VIDEO_PANEL_LVDS) += panel-lvds.o
obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_PANEL) += simple-panel.o
obj-$(CONFIG_DRIVER_VIDEO_MIPI_DBI) += mipi_dbi.o
obj-$(CONFIG_DRIVER_VIDEO_MIPI_DSI) += mipi_dsi.o
diff --git a/drivers/video/panel-lvds.c b/drivers/video/panel-lvds.c
new file mode 100644
index 0000000000..80d0b105ba
--- /dev/null
+++ b/drivers/video/panel-lvds.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * LVDS panel driver for barebox
+ *
+ * Supports "panel-lvds" compatible panels with a "panel-timing" subnode.
+ *
+ * Based on Linux drivers/gpu/drm/panel/panel-lvds.c
+ * Copyright (C) 2017 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Copyright Leica Geosystems AG
+ */
+
+#include <common.h>
+#include <init.h>
+#include <driver.h>
+#include <linux/err.h>
+#include <of.h>
+#include <fb.h>
+#include <regulator.h>
+#include <gpio.h>
+#include <of_gpio.h>
+#include <video/vpl.h>
+#include <video/backlight.h>
+
+struct panel_lvds {
+ struct device *dev;
+ struct vpl vpl;
+ struct regulator *power;
+ int enable_gpio;
+ enum of_gpio_flags enable_gpio_flags;
+ struct device_node *backlight_node;
+ struct backlight_device *backlight;
+ struct fb_videomode mode;
+ bool mode_valid;
+};
+
+static int panel_lvds_enable(struct panel_lvds *panel)
+{
+ int ret;
+
+ if (panel->power) {
+ ret = regulator_enable(panel->power);
+ if (ret) {
+ dev_err(panel->dev, "power-supply enable failed: %pe\n",
+ ERR_PTR(ret));
+ return ret;
+ }
+ }
+
+ if (gpio_is_valid(panel->enable_gpio)) {
+ int val = !(panel->enable_gpio_flags & OF_GPIO_ACTIVE_LOW);
+
+ gpio_direction_output(panel->enable_gpio, val);
+ }
+
+ /* Lazily resolve backlight on first enable (may probe after panel) */
+ if (panel->backlight_node && !panel->backlight)
+ panel->backlight = of_backlight_find(panel->backlight_node);
+
+ if (panel->backlight)
+ backlight_set_brightness_default(panel->backlight);
+
+ return 0;
+}
+
+static void panel_lvds_disable(struct panel_lvds *panel)
+{
+ if (panel->backlight)
+ backlight_set_brightness(panel->backlight, 0);
+
+ if (gpio_is_valid(panel->enable_gpio))
+ gpio_direction_output(panel->enable_gpio,
+ !!(panel->enable_gpio_flags & OF_GPIO_ACTIVE_LOW));
+
+ if (panel->power)
+ regulator_disable(panel->power);
+}
+
+static int panel_lvds_ioctl(struct vpl *vpl, unsigned int port,
+ unsigned int cmd, void *ptr)
+{
+ struct panel_lvds *panel = container_of(vpl, struct panel_lvds, vpl);
+ struct display_timings *timings;
+
+ switch (cmd) {
+ case VPL_ENABLE:
+ return panel_lvds_enable(panel);
+
+ case VPL_DISABLE:
+ panel_lvds_disable(panel);
+ return 0;
+
+ case VPL_PREPARE:
+ case VPL_UNPREPARE:
+ return 0;
+
+ case VPL_GET_VIDEOMODES:
+ timings = ptr;
+ if (!panel->mode_valid)
+ return -ENOENT;
+ timings->modes = &panel->mode;
+ timings->num_modes = 1;
+ return 0;
+
+ default:
+ return 0;
+ }
+}
+
+static int panel_lvds_probe(struct device *dev)
+{
+ struct panel_lvds *panel;
+ int ret;
+
+ panel = xzalloc(sizeof(*panel));
+ panel->dev = dev;
+
+ /* Parse panel-timing subnode */
+ ret = of_get_display_timing(dev->of_node, "panel-timing", &panel->mode);
+ if (ret) {
+ dev_err(dev, "failed to parse panel-timing: %d\n", ret);
+ return ret;
+ }
+ panel->mode_valid = true;
+
+ panel->power = regulator_get(dev, "power");
+ if (IS_ERR(panel->power)) {
+ /*
+ * power-supply is optional; if not present continue without it.
+ * This matches the Linux driver behavior.
+ */
+ if (PTR_ERR(panel->power) == -ENODEV)
+ panel->power = NULL;
+ else
+ return dev_errp_probe(dev, panel->power, "power supply\n");
+ }
+
+ panel->enable_gpio = of_get_named_gpio_flags(dev->of_node, "enable-gpios",
+ 0, &panel->enable_gpio_flags);
+ if (gpio_is_valid(panel->enable_gpio)) {
+ int initial = (panel->enable_gpio_flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
+
+ ret = gpio_request(panel->enable_gpio, "panel-enable");
+ if (ret)
+ dev_warn(dev, "failed to request enable-gpio: %d\n", ret);
+ else
+ gpio_direction_output(panel->enable_gpio, initial);
+ }
+
+ panel->backlight_node = of_parse_phandle(dev->of_node, "backlight", 0);
+
+ panel->vpl.node = dev->of_node;
+ panel->vpl.ioctl = panel_lvds_ioctl;
+
+ ret = vpl_register(&panel->vpl);
+ if (ret)
+ return ret;
+
+ dev_dbg(dev, "LVDS panel %dx%d registered\n",
+ panel->mode.xres, panel->mode.yres);
+ return 0;
+}
+
+static const struct of_device_id panel_lvds_dt_ids[] = {
+ { .compatible = "panel-lvds" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, panel_lvds_dt_ids);
+
+static struct driver panel_lvds_driver = {
+ .name = "panel-lvds",
+ .probe = panel_lvds_probe,
+ .of_compatible = DRV_OF_COMPAT(panel_lvds_dt_ids),
+};
+device_platform_driver(panel_lvds_driver);
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/6] video: add generic panel-lvds driver
2026-06-02 4:09 ` [PATCH 5/6] video: add generic panel-lvds driver Johannes Schneider
@ 2026-06-02 7:12 ` Ahmad Fatoum
0 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2026-06-02 7:12 UTC (permalink / raw)
To: Johannes Schneider, barebox; +Cc: thomas.haemmerle
Hello,
On 6/2/26 6:09 AM, Johannes Schneider wrote:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> Barebox has no driver for "panel-lvds" compatible DT nodes, which are used by
> all i.MX8M board display pipelines to describe LVDS panels with a panel-timing
> subnode. Without this driver the VPL chain in fsl-ldb terminates before it can
> query video modes, and the LCDIF2 framebuffer driver cannot determine the active
> display resolution, so no framebuffer is allocated and no splash is shown.
>
> The driver reads one video mode from the panel-timing subnode, enables
> an optional power-supply regulator and enable GPIO on VPL_ENABLE, and
> controls an optional backlight device. Closely follows the Linux
> drivers/gpu/drm/panel/panel-lvds.c behavior.
We have a driver though for simple-panel. I lean towards extending the
simple-panel driver with the panel-lvds compatible and adding any
feature it might miss there.
AFAICS, there is nothing really lvds specific here. I know Linux has
separate simple-panel drivers, but the barebox simple panel driver is
different: It has no hardcoded panel types, but only parses the DT,
exactly like what you do here.
> +static int panel_lvds_enable(struct panel_lvds *panel)
> +{
> + int ret;
> +
> + if (panel->power) {
> + ret = regulator_enable(panel->power);
regulator_enable(NULL) returns 0, so no need to check here.
> + panel->enable_gpio = of_get_named_gpio_flags(dev->of_node, "enable-gpios",
> + 0, &panel->enable_gpio_flags);
Please use the gpiod_ family of functions for new code.
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 4:09 [PATCH 0/7] video: enable boot splash on i.MX8MP with LVDS panel Johannes Schneider
` (4 preceding siblings ...)
2026-06-02 4:09 ` [PATCH 5/6] video: add generic panel-lvds driver Johannes Schneider
@ 2026-06-02 4:09 ` Johannes Schneider
2026-06-02 8:12 ` Lucas Stach
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schneider @ 2026-06-02 4:09 UTC (permalink / raw)
To: barebox; +Cc: thomas.haemmerle
From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
memory. After the splash command calls gu_screen_blit(), which copies the
decoded PNG via memcpy() from a cached shadow buffer into the hardware
framebuffer, writes may still reside in the CPU write buffer and not yet
be visible to the LCDIF DMA engine. On hardware this manifests as
partial or corrupted rendering — the bottom portion of the splash image
renders black or shows garbage.
Implement fb_damage() to execute a DSB SY barrier after each blit,
ensuring all pending write-combine writes are committed to DRAM before
the LCDIF reads the pixel data.
Assisted-by: Claude:claude-sonnet-4-6
Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
---
drivers/video/imx-lcdif.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
index ae5976c771..8d9b40be0f 100644
--- a/drivers/video/imx-lcdif.c
+++ b/drivers/video/imx-lcdif.c
@@ -20,6 +20,7 @@
#include <of_graph.h>
#include <video/media-bus-format.h>
#include <video/vpl.h>
+#include <asm/system.h>
/* LCDIF V8 register map */
#define LCDC_V8_CTRL 0x00
@@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
clk_disable_unprepare(priv->clk_axi);
}
+static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
+{
+ /*
+ * The hardware framebuffer is Normal Non-Cacheable (write-combine).
+ * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
+ * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
+ * drain the write buffer to DRAM before the LCDIF reads the pixels.
+ */
+ dsb();
+}
+
static struct fb_ops lcdif_fb_ops = {
.fb_enable = lcdif_fb_enable,
.fb_disable = lcdif_fb_disable,
+ .fb_damage = lcdif_fb_damage,
};
static int lcdif_probe(struct device *dev)
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 4:09 ` [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels Johannes Schneider
@ 2026-06-02 8:12 ` Lucas Stach
2026-06-02 9:37 ` Ahmad Fatoum
0 siblings, 1 reply; 19+ messages in thread
From: Lucas Stach @ 2026-06-02 8:12 UTC (permalink / raw)
To: Johannes Schneider, barebox; +Cc: thomas.haemmerle
Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>
> The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
> memory. After the splash command calls gu_screen_blit(), which copies the
> decoded PNG via memcpy() from a cached shadow buffer into the hardware
> framebuffer, writes may still reside in the CPU write buffer and not yet
> be visible to the LCDIF DMA engine. On hardware this manifests as
> partial or corrupted rendering — the bottom portion of the splash image
> renders black or shows garbage.
>
> Implement fb_damage() to execute a DSB SY barrier after each blit,
> ensuring all pending write-combine writes are committed to DRAM before
> the LCDIF reads the pixel data.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> ---
> drivers/video/imx-lcdif.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> index ae5976c771..8d9b40be0f 100644
> --- a/drivers/video/imx-lcdif.c
> +++ b/drivers/video/imx-lcdif.c
> @@ -20,6 +20,7 @@
> #include <of_graph.h>
> #include <video/media-bus-format.h>
> #include <video/vpl.h>
> +#include <asm/system.h>
>
> /* LCDIF V8 register map */
> #define LCDC_V8_CTRL 0x00
> @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
> clk_disable_unprepare(priv->clk_axi);
> }
>
> +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
> +{
> + /*
> + * The hardware framebuffer is Normal Non-Cacheable (write-combine).
> + * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
> + * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
> + * drain the write buffer to DRAM before the LCDIF reads the pixels.
> + */
> + dsb();
This will probably work in practice, but it doesn't exactly do what the
comment above states. A dsb does not drain the write combine buffers,
it just makes sure that they are drained before the next instruction is
executed.
So if the fb_damage is the last thing that gets executed, this will not
ensure that the writes will actually become visible to external agents.
A more robust way to drain the write combine buffers is to execute a
readback of a memory location within the WC region.
Regards,
Lucas
> +}
> +
> static struct fb_ops lcdif_fb_ops = {
> .fb_enable = lcdif_fb_enable,
> .fb_disable = lcdif_fb_disable,
> + .fb_damage = lcdif_fb_damage,
> };
>
> static int lcdif_probe(struct device *dev)
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 8:12 ` Lucas Stach
@ 2026-06-02 9:37 ` Ahmad Fatoum
2026-06-02 12:12 ` Lucas Stach
0 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2026-06-02 9:37 UTC (permalink / raw)
To: Lucas Stach, Johannes Schneider, barebox; +Cc: thomas.haemmerle
On 6/2/26 10:12 AM, Lucas Stach wrote:
> Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
>> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>>
>> The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
>> memory. After the splash command calls gu_screen_blit(), which copies the
>> decoded PNG via memcpy() from a cached shadow buffer into the hardware
>> framebuffer, writes may still reside in the CPU write buffer and not yet
>> be visible to the LCDIF DMA engine. On hardware this manifests as
>> partial or corrupted rendering — the bottom portion of the splash image
>> renders black or shows garbage.
>>
>> Implement fb_damage() to execute a DSB SY barrier after each blit,
>> ensuring all pending write-combine writes are committed to DRAM before
>> the LCDIF reads the pixel data.
>>
>> Assisted-by: Claude:claude-sonnet-4-6
>> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
>> ---
>> drivers/video/imx-lcdif.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
>> index ae5976c771..8d9b40be0f 100644
>> --- a/drivers/video/imx-lcdif.c
>> +++ b/drivers/video/imx-lcdif.c
>> @@ -20,6 +20,7 @@
>> #include <of_graph.h>
>> #include <video/media-bus-format.h>
>> #include <video/vpl.h>
>> +#include <asm/system.h>
>>
>> /* LCDIF V8 register map */
>> #define LCDC_V8_CTRL 0x00
>> @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
>> clk_disable_unprepare(priv->clk_axi);
>> }
>>
>> +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
>> +{
>> + /*
>> + * The hardware framebuffer is Normal Non-Cacheable (write-combine).
>> + * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
>> + * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
>> + * drain the write buffer to DRAM before the LCDIF reads the pixels.
>> + */
>> + dsb();
>
> This will probably work in practice, but it doesn't exactly do what the
> comment above states. A dsb does not drain the write combine buffers,
> it just makes sure that they are drained before the next instruction is
> executed.
> So if the fb_damage is the last thing that gets executed, this will not
> ensure that the writes will actually become visible to external agents.
>
> A more robust way to drain the write combine buffers is to execute a
> readback of a memory location within the WC region.
Would that on its own suffice though? If we have just the single buffer,
flushing the write buffer only reduces the race window, but doesn't
remove it as the framebuffer write may still happen in parallel to the
scanout.
Cheers,
Ahmad
>
> Regards,
> Lucas
>> +}
>> +
>> static struct fb_ops lcdif_fb_ops = {
>> .fb_enable = lcdif_fb_enable,
>> .fb_disable = lcdif_fb_disable,
>> + .fb_damage = lcdif_fb_damage,
>> };
>>
>> static int lcdif_probe(struct device *dev)
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 9:37 ` Ahmad Fatoum
@ 2026-06-02 12:12 ` Lucas Stach
2026-06-02 15:17 ` Michael Grzeschik
2026-06-02 17:10 ` Michael Grzeschik
0 siblings, 2 replies; 19+ messages in thread
From: Lucas Stach @ 2026-06-02 12:12 UTC (permalink / raw)
To: Ahmad Fatoum, Johannes Schneider, barebox; +Cc: thomas.haemmerle
Am Dienstag, dem 02.06.2026 um 11:37 +0200 schrieb Ahmad Fatoum:
>
> On 6/2/26 10:12 AM, Lucas Stach wrote:
> > Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
> > > From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > >
> > > The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
> > > memory. After the splash command calls gu_screen_blit(), which copies the
> > > decoded PNG via memcpy() from a cached shadow buffer into the hardware
> > > framebuffer, writes may still reside in the CPU write buffer and not yet
> > > be visible to the LCDIF DMA engine. On hardware this manifests as
> > > partial or corrupted rendering — the bottom portion of the splash image
> > > renders black or shows garbage.
> > >
> > > Implement fb_damage() to execute a DSB SY barrier after each blit,
> > > ensuring all pending write-combine writes are committed to DRAM before
> > > the LCDIF reads the pixel data.
> > >
> > > Assisted-by: Claude:claude-sonnet-4-6
> > > Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > ---
> > > drivers/video/imx-lcdif.c | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> > > index ae5976c771..8d9b40be0f 100644
> > > --- a/drivers/video/imx-lcdif.c
> > > +++ b/drivers/video/imx-lcdif.c
> > > @@ -20,6 +20,7 @@
> > > #include <of_graph.h>
> > > #include <video/media-bus-format.h>
> > > #include <video/vpl.h>
> > > +#include <asm/system.h>
> > >
> > > /* LCDIF V8 register map */
> > > #define LCDC_V8_CTRL 0x00
> > > @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
> > > clk_disable_unprepare(priv->clk_axi);
> > > }
> > >
> > > +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
> > > +{
> > > + /*
> > > + * The hardware framebuffer is Normal Non-Cacheable (write-combine).
> > > + * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
> > > + * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
> > > + * drain the write buffer to DRAM before the LCDIF reads the pixels.
> > > + */
> > > + dsb();
> >
> > This will probably work in practice, but it doesn't exactly do what the
> > comment above states. A dsb does not drain the write combine buffers,
> > it just makes sure that they are drained before the next instruction is
> > executed.
> > So if the fb_damage is the last thing that gets executed, this will not
> > ensure that the writes will actually become visible to external agents.
> >
> > A more robust way to drain the write combine buffers is to execute a
> > readback of a memory location within the WC region.
>
> Would that on its own suffice though? If we have just the single buffer,
> flushing the write buffer only reduces the race window, but doesn't
> remove it as the framebuffer write may still happen in parallel to the
> scanout.
Updating the (front-)framebuffer will always be racy against the
scanout. The flushes just ensure that the writes eventually reach
memory visible to the scanout DMA master. Without any flushes there is
nothing in the memory model which would prohibit writes from being
stuck in the write buffers indefinitely.
Regards,
Lucas
>
> Cheers,
> Ahmad
>
> >
> > Regards,
> > Lucas
> > > +}
> > > +
> > > static struct fb_ops lcdif_fb_ops = {
> > > .fb_enable = lcdif_fb_enable,
> > > .fb_disable = lcdif_fb_disable,
> > > + .fb_damage = lcdif_fb_damage,
> > > };
> > >
> > > static int lcdif_probe(struct device *dev)
> >
> >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 12:12 ` Lucas Stach
@ 2026-06-02 15:17 ` Michael Grzeschik
2026-06-02 17:10 ` Michael Grzeschik
1 sibling, 0 replies; 19+ messages in thread
From: Michael Grzeschik @ 2026-06-02 15:17 UTC (permalink / raw)
To: Lucas Stach; +Cc: Ahmad Fatoum, Johannes Schneider, barebox, thomas.haemmerle
Hi Johannes
On Tue, Jun 02, 2026 at 02:12:43PM +0200, Lucas Stach wrote:
> Am Dienstag, dem 02.06.2026 um 11:37 +0200 schrieb Ahmad Fatoum:
> >
> > On 6/2/26 10:12 AM, Lucas Stach wrote:
> > > Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
> > > > From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > >
> > > > The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
> > > > memory. After the splash command calls gu_screen_blit(), which copies the
> > > > decoded PNG via memcpy() from a cached shadow buffer into the hardware
> > > > framebuffer, writes may still reside in the CPU write buffer and not yet
> > > > be visible to the LCDIF DMA engine. On hardware this manifests as
> > > > partial or corrupted rendering — the bottom portion of the splash image
> > > > renders black or shows garbage.
> > > >
> > > > Implement fb_damage() to execute a DSB SY barrier after each blit,
> > > > ensuring all pending write-combine writes are committed to DRAM before
> > > > the LCDIF reads the pixel data.
> > > >
> > > > Assisted-by: Claude:claude-sonnet-4-6
> > > > Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > > ---
> > > > drivers/video/imx-lcdif.c | 13 +++++++++++++
> > > > 1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> > > > index ae5976c771..8d9b40be0f 100644
> > > > --- a/drivers/video/imx-lcdif.c
> > > > +++ b/drivers/video/imx-lcdif.c
> > > > @@ -20,6 +20,7 @@
> > > > #include <of_graph.h>
> > > > #include <video/media-bus-format.h>
> > > > #include <video/vpl.h>
> > > > +#include <asm/system.h>
> > > >
> > > > /* LCDIF V8 register map */
> > > > #define LCDC_V8_CTRL 0x00
> > > > @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
> > > > clk_disable_unprepare(priv->clk_axi);
> > > > }
> > > >
> > > > +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
> > > > +{
> > > > + /*
> > > > + * The hardware framebuffer is Normal Non-Cacheable (write-combine).
> > > > + * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
> > > > + * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
> > > > + * drain the write buffer to DRAM before the LCDIF reads the pixels.
> > > > + */
> > > > + dsb();
> > >
> > > This will probably work in practice, but it doesn't exactly do what the
> > > comment above states. A dsb does not drain the write combine buffers,
> > > it just makes sure that they are drained before the next instruction is
> > > executed.
> > > So if the fb_damage is the last thing that gets executed, this will not
> > > ensure that the writes will actually become visible to external agents.
> > >
> > > A more robust way to drain the write combine buffers is to execute a
> > > readback of a memory location within the WC region.
> >
> > Would that on its own suffice though? If we have just the single buffer,
> > flushing the write buffer only reduces the race window, but doesn't
> > remove it as the framebuffer write may still happen in parallel to the
> > scanout.
>
> Updating the (front-)framebuffer will always be racy against the
> scanout. The flushes just ensure that the writes eventually reach
> memory visible to the scanout DMA master. Without any flushes there is
> nothing in the memory model which would prohibit writes from being
> stuck in the write buffers indefinitely.
I wonder why this all is actually necessary. The driver has an shadowed
buffer which can be flushed when ready.
Your implementation already has this comment.
+ /* 32bpp XRGB8888 → ARGB8888 pixel format.
+ * SHADOW_LOAD_EN causes the descriptor registers (address, format, pitch)
+ * to be latched into the active registers on the next VSYNC, which is
+ * required for the initial descriptor load in barebox's single-enable
+ * sequence (no DRM page-flip mechanism). EN is ORed in later by
+ * lcdif_enable() after DISP_ON. */
Look into mine and you will find the function flush function, which will
exactly do this after every frame write.
> > > > +}
> > > > +
> > > > static struct fb_ops lcdif_fb_ops = {
> > > > .fb_enable = lcdif_fb_enable,
> > > > .fb_disable = lcdif_fb_disable,
> > > > + .fb_damage = lcdif_fb_damage,
> > > > };
> > > >
> > > > static int lcdif_probe(struct device *dev)
> > >
> > >
Regards,
Michael
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 12:12 ` Lucas Stach
2026-06-02 15:17 ` Michael Grzeschik
@ 2026-06-02 17:10 ` Michael Grzeschik
2026-06-04 3:42 ` SCHNEIDER Johannes
1 sibling, 1 reply; 19+ messages in thread
From: Michael Grzeschik @ 2026-06-02 17:10 UTC (permalink / raw)
To: Lucas Stach; +Cc: Ahmad Fatoum, Johannes Schneider, barebox, thomas.haemmerle
On Tue, Jun 02, 2026 at 02:12:43PM +0200, Lucas Stach wrote:
> Am Dienstag, dem 02.06.2026 um 11:37 +0200 schrieb Ahmad Fatoum:
> >
> > On 6/2/26 10:12 AM, Lucas Stach wrote:
> > > Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
> > > > From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > >
> > > > The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
> > > > memory. After the splash command calls gu_screen_blit(), which copies the
> > > > decoded PNG via memcpy() from a cached shadow buffer into the hardware
> > > > framebuffer, writes may still reside in the CPU write buffer and not yet
> > > > be visible to the LCDIF DMA engine. On hardware this manifests as
> > > > partial or corrupted rendering — the bottom portion of the splash image
> > > > renders black or shows garbage.
> > > >
> > > > Implement fb_damage() to execute a DSB SY barrier after each blit,
> > > > ensuring all pending write-combine writes are committed to DRAM before
> > > > the LCDIF reads the pixel data.
> > > >
> > > > Assisted-by: Claude:claude-sonnet-4-6
> > > > Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > > ---
> > > > drivers/video/imx-lcdif.c | 13 +++++++++++++
> > > > 1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> > > > index ae5976c771..8d9b40be0f 100644
> > > > --- a/drivers/video/imx-lcdif.c
> > > > +++ b/drivers/video/imx-lcdif.c
> > > > @@ -20,6 +20,7 @@
> > > > #include <of_graph.h>
> > > > #include <video/media-bus-format.h>
> > > > #include <video/vpl.h>
> > > > +#include <asm/system.h>
> > > >
> > > > /* LCDIF V8 register map */
> > > > #define LCDC_V8_CTRL 0x00
> > > > @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
> > > > clk_disable_unprepare(priv->clk_axi);
> > > > }
> > > >
> > > > +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
> > > > +{
> > > > + /*
> > > > + * The hardware framebuffer is Normal Non-Cacheable (write-combine).
> > > > + * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
> > > > + * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
> > > > + * drain the write buffer to DRAM before the LCDIF reads the pixels.
> > > > + */
> > > > + dsb();
> > >
> > > This will probably work in practice, but it doesn't exactly do what the
> > > comment above states. A dsb does not drain the write combine buffers,
> > > it just makes sure that they are drained before the next instruction is
> > > executed.
> > > So if the fb_damage is the last thing that gets executed, this will not
> > > ensure that the writes will actually become visible to external agents.
> > >
> > > A more robust way to drain the write combine buffers is to execute a
> > > readback of a memory location within the WC region.
> >
> > Would that on its own suffice though? If we have just the single buffer,
> > flushing the write buffer only reduces the race window, but doesn't
> > remove it as the framebuffer write may still happen in parallel to the
> > scanout.
>
> Updating the (front-)framebuffer will always be racy against the
> scanout. The flushes just ensure that the writes eventually reach
> memory visible to the scanout DMA master. Without any flushes there is
> nothing in the memory model which would prohibit writes from being
> stuck in the write buffers indefinitely.
I am curious if this is actually all necessary.
In the driver Johannes send, there is the comment
+ /* 32bpp XRGB8888 → ARGB8888 pixel format.
+ * SHADOW_LOAD_EN causes the descriptor registers (address, format, pitch)
+ * to be latched into the active registers on the next VSYNC, which is
+ * required for the initial descriptor load in barebox's single-enable
+ * sequence (no DRM page-flip mechanism). EN is ORed in later by
+ * lcdif_enable() after DISP_ON. */
in lcdif_set_plane of the imx-lcdif driver.
This exactly flushes the buffer once. I also did this but also
implemented lcdif_crtc_atomic_flush in drivers/video/lcdif_kms.c as the
fb_flush callback which will do exactly this LOAD_EN call every time a
new frame was written. With this change I had no issues with distorted
images.
> > > > +}
> > > > +
> > > > static struct fb_ops lcdif_fb_ops = {
> > > > .fb_enable = lcdif_fb_enable,
> > > > .fb_disable = lcdif_fb_disable,
> > > > + .fb_damage = lcdif_fb_damage,
> > > > };
> > > >
> > > > static int lcdif_probe(struct device *dev)
> > >
> > >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels
2026-06-02 17:10 ` Michael Grzeschik
@ 2026-06-04 3:42 ` SCHNEIDER Johannes
0 siblings, 0 replies; 19+ messages in thread
From: SCHNEIDER Johannes @ 2026-06-04 3:42 UTC (permalink / raw)
To: Michael Grzeschik, Lucas Stach, Ahmad Fatoum; +Cc: barebox, HAEMMERLE Thomas
Hoi all,
> On Tue, Jun 02, 2026 at 02:12:43PM +0200, Lucas Stach wrote:
> > Am Dienstag, dem 02.06.2026 um 11:37 +0200 schrieb Ahmad Fatoum:
> > >
> > > On 6/2/26 10:12 AM, Lucas Stach wrote:
> > > > Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
> > > > > From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > > >
> > > > > The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
> > > > > memory. After the splash command calls gu_screen_blit(), which copies the
> > > > > decoded PNG via memcpy() from a cached shadow buffer into the hardware
> > > > > framebuffer, writes may still reside in the CPU write buffer and not yet
> > > > > be visible to the LCDIF DMA engine. On hardware this manifests as
> > > > > partial or corrupted rendering — the bottom portion of the splash image
> > > > > renders black or shows garbage.
> > > > >
> > > > > Implement fb_damage() to execute a DSB SY barrier after each blit,
> > > > > ensuring all pending write-combine writes are committed to DRAM before
> > > > > the LCDIF reads the pixel data.
> > > > >
> > > > > Assisted-by: Claude:claude-sonnet-4-6
> > > > > Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> > > > > ---
> > > > > drivers/video/imx-lcdif.c | 13 +++++++++++++
> > > > > 1 file changed, 13 insertions(+)
> > > > >
> > > > > diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> > > > > index ae5976c771..8d9b40be0f 100644
> > > > > --- a/drivers/video/imx-lcdif.c
> > > > > +++ b/drivers/video/imx-lcdif.c
> > > > > @@ -20,6 +20,7 @@
> > > > > #include <of_graph.h>
> > > > > #include <video/media-bus-format.h>
> > > > > #include <video/vpl.h>
> > > > > +#include <asm/system.h>
> > > > >
> > > > > /* LCDIF V8 register map */
> > > > > #define LCDC_V8_CTRL 0x00
> > > > > @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
> > > > > clk_disable_unprepare(priv->clk_axi);
> > > > > }
> > > > >
> > > > > +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
> > > > > +{
> > > > > + /*
> > > > > + * The hardware framebuffer is Normal Non-Cacheable (write-combine).
> > > > > + * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
> > > > > + * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
> > > > > + * drain the write buffer to DRAM before the LCDIF reads the pixels.
> > > > > + */
> > > > > + dsb();
> > > >
> > > > This will probably work in practice, but it doesn't exactly do what the
> > > > comment above states. A dsb does not drain the write combine buffers,
> > > > it just makes sure that they are drained before the next instruction is
> > > > executed.
> > > > So if the fb_damage is the last thing that gets executed, this will not
> > > > ensure that the writes will actually become visible to external agents.
> > > >
> > > > A more robust way to drain the write combine buffers is to execute a
> > > > readback of a memory location within the WC region.
> > >
> > > Would that on its own suffice though? If we have just the single buffer,
> > > flushing the write buffer only reduces the race window, but doesn't
> > > remove it as the framebuffer write may still happen in parallel to the
> > > scanout.
> >
> > Updating the (front-)framebuffer will always be racy against the
> > scanout. The flushes just ensure that the writes eventually reach
> > memory visible to the scanout DMA master. Without any flushes there is
> > nothing in the memory model which would prohibit writes from being
> > stuck in the write buffers indefinitely.
Thanks for all the input, since Ahmad also pointed out that there is already a
driver, i'll restructure this patchstack around the upstream on and weave in all
the input so far -> expect a v2.
>
> I am curious if this is actually all necessary.
>
i'll admit that i don't have a very deep understanding of the whole pixel
pipeline, and am just putting/copying (from the kernel mostly) things together
and "massaging" them until the display showed something (without any weird
distortions or other artifacts)
> In the driver Johannes send, there is the comment
>
> + /* 32bpp XRGB8888 → ARGB8888 pixel format.
> + * SHADOW_LOAD_EN causes the descriptor registers (address, format, pitch)
> + * to be latched into the active registers on the next VSYNC, which is
> + * required for the initial descriptor load in barebox's single-enable
> + * sequence (no DRM page-flip mechanism). EN is ORed in later by
> + * lcdif_enable() after DISP_ON. */
>
> in lcdif_set_plane of the imx-lcdif driver.
>
> This exactly flushes the buffer once. I also did this but also
> implemented lcdif_crtc_atomic_flush in drivers/video/lcdif_kms.c as the
> fb_flush callback which will do exactly this LOAD_EN call every time a
> new frame was written. With this change I had no issues with distorted
> images.
>
> > > > > +}
> > > > > +
> > > > > static struct fb_ops lcdif_fb_ops = {
> > > > > .fb_enable = lcdif_fb_enable,
> > > > > .fb_disable = lcdif_fb_disable,
> > > > > + .fb_damage = lcdif_fb_damage,
> > > > > };
> > > > >
> > > > > static int lcdif_probe(struct device *dev)
> > > >
> > > >
>
gruß
Johannes
^ permalink raw reply [flat|nested] 19+ messages in thread