* [PATCH v3] drm/bridge: fsl-ldb: Parse register offsets from DT
@ 2026-01-04 21:36 Marek Vasut
2026-01-13 16:39 ` Marco Felsch
0 siblings, 1 reply; 2+ messages in thread
From: Marek Vasut @ 2026-01-04 21:36 UTC (permalink / raw)
To: dri-devel
Cc: Marek Vasut, Abel Vesa, Conor Dooley, Fabio Estevam,
Krzysztof Kozlowski, Laurent Pinchart, Liu Ying, Luca Ceresoli,
Lucas Stach, Peng Fan, Pengutronix Kernel Team, Rob Herring,
Shawn Guo, Thomas Zimmermann, devicetree, imx, linux-arm-kernel,
linux-clk
The DT bindings for this bridge describe register offsets for the LDB,
parse the register offsets from DT instead of hard-coding them in the
driver. No functional change.
The LDB was always meant to parse the 'reg' offsets out of the DT, it
only went somewhat wrong in the process and we ended up with hard-coded
reg<->compatible mapping. It was never intended to be that way, so fix
it.
Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
---
Cc: Abel Vesa <abelvesa@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Liu Ying <victor.liu@nxp.com>
Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-clk@vger.kernel.org
---
V2: - Switch to of_property_read_reg()
- Parse single-register LDB variants from DT too
V3: - Update commit message
- Drop "likely" from the comment
---
drivers/gpu/drm/bridge/fsl-ldb.c | 58 ++++++++++++++++++++------------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index 5c3cf37200bce..aa352c70b9ab2 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -8,6 +8,7 @@
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/of_graph.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -61,24 +62,13 @@ enum fsl_ldb_devtype {
};
struct fsl_ldb_devdata {
- u32 ldb_ctrl;
- u32 lvds_ctrl;
bool lvds_en_bit;
- bool single_ctrl_reg;
};
static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
- [IMX6SX_LDB] = {
- .ldb_ctrl = 0x18,
- .single_ctrl_reg = true,
- },
- [IMX8MP_LDB] = {
- .ldb_ctrl = 0x5c,
- .lvds_ctrl = 0x128,
- },
+ [IMX6SX_LDB] = { },
+ [IMX8MP_LDB] = { },
[IMX93_LDB] = {
- .ldb_ctrl = 0x20,
- .lvds_ctrl = 0x24,
.lvds_en_bit = true,
},
};
@@ -90,8 +80,11 @@ struct fsl_ldb {
struct clk *clk;
struct regmap *regmap;
const struct fsl_ldb_devdata *devdata;
+ u64 ldb_ctrl;
+ u64 lvds_ctrl;
bool ch0_enabled;
bool ch1_enabled;
+ bool single_ctrl_reg;
};
static bool fsl_ldb_is_dual(const struct fsl_ldb *fsl_ldb)
@@ -204,15 +197,15 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
reg |= (fsl_ldb->ch0_enabled ? LDB_CTRL_DI0_VSYNC_POLARITY : 0) |
(fsl_ldb->ch1_enabled ? LDB_CTRL_DI1_VSYNC_POLARITY : 0);
- regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, reg);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->ldb_ctrl, reg);
- if (fsl_ldb->devdata->single_ctrl_reg)
+ if (fsl_ldb->single_ctrl_reg)
return;
/* Program LVDS_CTRL */
reg = LVDS_CTRL_CC_ADJ(2) | LVDS_CTRL_PRE_EMPH_EN |
LVDS_CTRL_PRE_EMPH_ADJ(3) | LVDS_CTRL_VBG_EN;
- regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, reg);
/* Wait for VBG to stabilize. */
usleep_range(15, 20);
@@ -220,7 +213,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
reg |= (fsl_ldb->ch0_enabled ? LVDS_CTRL_CH0_EN : 0) |
(fsl_ldb->ch1_enabled ? LVDS_CTRL_CH1_EN : 0);
- regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, reg);
}
static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
@@ -231,12 +224,12 @@ static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
/* Stop channel(s). */
if (fsl_ldb->devdata->lvds_en_bit)
/* Set LVDS_CTRL_LVDS_EN bit to disable. */
- regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl,
+ regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl,
LVDS_CTRL_LVDS_EN);
else
- if (!fsl_ldb->devdata->single_ctrl_reg)
- regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, 0);
- regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, 0);
+ if (!fsl_ldb->single_ctrl_reg)
+ regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, 0);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->ldb_ctrl, 0);
clk_disable_unprepare(fsl_ldb->clk);
}
@@ -296,7 +289,7 @@ static int fsl_ldb_probe(struct platform_device *pdev)
struct device_node *remote1, *remote2;
struct drm_panel *panel;
struct fsl_ldb *fsl_ldb;
- int dual_link;
+ int dual_link, idx, ret;
fsl_ldb = devm_drm_bridge_alloc(dev, struct fsl_ldb, bridge, &funcs);
if (IS_ERR(fsl_ldb))
@@ -309,6 +302,27 @@ static int fsl_ldb_probe(struct platform_device *pdev)
fsl_ldb->dev = &pdev->dev;
fsl_ldb->bridge.of_node = dev->of_node;
+ /* No "reg-names" property means single-register LDB */
+ idx = of_property_match_string(dev->of_node, "reg-names", "ldb");
+ if (idx < 0) {
+ fsl_ldb->single_ctrl_reg = true;
+ idx = 0;
+ }
+
+ ret = of_property_read_reg(dev->of_node, idx, &fsl_ldb->ldb_ctrl, NULL);
+ if (ret)
+ return ret;
+
+ if (!fsl_ldb->single_ctrl_reg) {
+ idx = of_property_match_string(dev->of_node, "reg-names", "lvds");
+ if (idx < 0)
+ return idx;
+
+ ret = of_property_read_reg(dev->of_node, idx, &fsl_ldb->lvds_ctrl, NULL);
+ if (ret)
+ return ret;
+ }
+
fsl_ldb->clk = devm_clk_get(dev, "ldb");
if (IS_ERR(fsl_ldb->clk))
return PTR_ERR(fsl_ldb->clk);
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] drm/bridge: fsl-ldb: Parse register offsets from DT
2026-01-04 21:36 [PATCH v3] drm/bridge: fsl-ldb: Parse register offsets from DT Marek Vasut
@ 2026-01-13 16:39 ` Marco Felsch
0 siblings, 0 replies; 2+ messages in thread
From: Marco Felsch @ 2026-01-13 16:39 UTC (permalink / raw)
To: Marek Vasut
Cc: dri-devel, Rob Herring, Conor Dooley, Thomas Zimmermann,
devicetree, Luca Ceresoli, Peng Fan, Liu Ying, imx,
Laurent Pinchart, Pengutronix Kernel Team, Shawn Guo,
Krzysztof Kozlowski, Lucas Stach, Fabio Estevam, linux-clk,
linux-arm-kernel, Abel Vesa
Hi Marek,
On 26-01-04, Marek Vasut wrote:
> The DT bindings for this bridge describe register offsets for the LDB,
> parse the register offsets from DT instead of hard-coding them in the
> driver. No functional change.
during the discussion of the i.MX9 PDFC bridge dt-bindings [1] we came
to the conclusion that the 'reg' property shouldn't have been added to
the dt-bindings in the first place.
[1] https://lore.kernel.org/all/20251219153537.zgxcokyhcqerw4jp@pengutronix.de/
As descibed in [1], the PDFC and LDB aren't standalone IPs with
dedicated register spaces. For some cases like the LDB, there may be two
dedicated registers, but for other cases like the PDFC, there is only a
single register-field and the register containing this field is shared
between the PDFC and the MIPI-DSI block.
Therefore the 'reg' DT abstraction is wrong, instead the BLK-CTRL child
devices should appear without a 'reg' property as simple child devices.
The child devices need to get the register space from the syscon parent
and need to code the registers and register-fields within the driver
like the LDB does currently.
Sorry for the late feedback.
Regards,
Marco
> The LDB was always meant to parse the 'reg' offsets out of the DT, it
> only went somewhat wrong in the process and we ended up with hard-coded
> reg<->compatible mapping. It was never intended to be that way, so fix
> it.
>
> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
> ---
> Cc: Abel Vesa <abelvesa@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-clk@vger.kernel.org
> ---
> V2: - Switch to of_property_read_reg()
> - Parse single-register LDB variants from DT too
> V3: - Update commit message
> - Drop "likely" from the comment
> ---
> drivers/gpu/drm/bridge/fsl-ldb.c | 58 ++++++++++++++++++++------------
> 1 file changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 5c3cf37200bce..aa352c70b9ab2 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -8,6 +8,7 @@
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/of_graph.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -61,24 +62,13 @@ enum fsl_ldb_devtype {
> };
>
> struct fsl_ldb_devdata {
> - u32 ldb_ctrl;
> - u32 lvds_ctrl;
> bool lvds_en_bit;
> - bool single_ctrl_reg;
> };
>
> static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
> - [IMX6SX_LDB] = {
> - .ldb_ctrl = 0x18,
> - .single_ctrl_reg = true,
> - },
> - [IMX8MP_LDB] = {
> - .ldb_ctrl = 0x5c,
> - .lvds_ctrl = 0x128,
> - },
> + [IMX6SX_LDB] = { },
> + [IMX8MP_LDB] = { },
> [IMX93_LDB] = {
> - .ldb_ctrl = 0x20,
> - .lvds_ctrl = 0x24,
> .lvds_en_bit = true,
> },
> };
> @@ -90,8 +80,11 @@ struct fsl_ldb {
> struct clk *clk;
> struct regmap *regmap;
> const struct fsl_ldb_devdata *devdata;
> + u64 ldb_ctrl;
> + u64 lvds_ctrl;
> bool ch0_enabled;
> bool ch1_enabled;
> + bool single_ctrl_reg;
> };
>
> static bool fsl_ldb_is_dual(const struct fsl_ldb *fsl_ldb)
> @@ -204,15 +197,15 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
> reg |= (fsl_ldb->ch0_enabled ? LDB_CTRL_DI0_VSYNC_POLARITY : 0) |
> (fsl_ldb->ch1_enabled ? LDB_CTRL_DI1_VSYNC_POLARITY : 0);
>
> - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, reg);
> + regmap_write(fsl_ldb->regmap, fsl_ldb->ldb_ctrl, reg);
>
> - if (fsl_ldb->devdata->single_ctrl_reg)
> + if (fsl_ldb->single_ctrl_reg)
> return;
>
> /* Program LVDS_CTRL */
> reg = LVDS_CTRL_CC_ADJ(2) | LVDS_CTRL_PRE_EMPH_EN |
> LVDS_CTRL_PRE_EMPH_ADJ(3) | LVDS_CTRL_VBG_EN;
> - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);
> + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, reg);
>
> /* Wait for VBG to stabilize. */
> usleep_range(15, 20);
> @@ -220,7 +213,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
> reg |= (fsl_ldb->ch0_enabled ? LVDS_CTRL_CH0_EN : 0) |
> (fsl_ldb->ch1_enabled ? LVDS_CTRL_CH1_EN : 0);
>
> - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);
> + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, reg);
> }
>
> static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
> @@ -231,12 +224,12 @@ static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
> /* Stop channel(s). */
> if (fsl_ldb->devdata->lvds_en_bit)
> /* Set LVDS_CTRL_LVDS_EN bit to disable. */
> - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl,
> + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl,
> LVDS_CTRL_LVDS_EN);
> else
> - if (!fsl_ldb->devdata->single_ctrl_reg)
> - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, 0);
> - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, 0);
> + if (!fsl_ldb->single_ctrl_reg)
> + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, 0);
> + regmap_write(fsl_ldb->regmap, fsl_ldb->ldb_ctrl, 0);
>
> clk_disable_unprepare(fsl_ldb->clk);
> }
> @@ -296,7 +289,7 @@ static int fsl_ldb_probe(struct platform_device *pdev)
> struct device_node *remote1, *remote2;
> struct drm_panel *panel;
> struct fsl_ldb *fsl_ldb;
> - int dual_link;
> + int dual_link, idx, ret;
>
> fsl_ldb = devm_drm_bridge_alloc(dev, struct fsl_ldb, bridge, &funcs);
> if (IS_ERR(fsl_ldb))
> @@ -309,6 +302,27 @@ static int fsl_ldb_probe(struct platform_device *pdev)
> fsl_ldb->dev = &pdev->dev;
> fsl_ldb->bridge.of_node = dev->of_node;
>
> + /* No "reg-names" property means single-register LDB */
> + idx = of_property_match_string(dev->of_node, "reg-names", "ldb");
> + if (idx < 0) {
> + fsl_ldb->single_ctrl_reg = true;
> + idx = 0;
> + }
> +
> + ret = of_property_read_reg(dev->of_node, idx, &fsl_ldb->ldb_ctrl, NULL);
> + if (ret)
> + return ret;
> +
> + if (!fsl_ldb->single_ctrl_reg) {
> + idx = of_property_match_string(dev->of_node, "reg-names", "lvds");
> + if (idx < 0)
> + return idx;
> +
> + ret = of_property_read_reg(dev->of_node, idx, &fsl_ldb->lvds_ctrl, NULL);
> + if (ret)
> + return ret;
> + }
> +
> fsl_ldb->clk = devm_clk_get(dev, "ldb");
> if (IS_ERR(fsl_ldb->clk))
> return PTR_ERR(fsl_ldb->clk);
> --
> 2.51.0
>
>
>
--
#gernperDu
#CallMeByMyFirstName
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-01-13 16:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-04 21:36 [PATCH v3] drm/bridge: fsl-ldb: Parse register offsets from DT Marek Vasut
2026-01-13 16:39 ` Marco Felsch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox