Hi Fabio,
On Thu, Dec 18, 2025 at 08:06:26AM -0300, Fabio Estevam wrote:
> The LVDS Control Register (LVDS_CTRL) register has an HS_EN bit that allows
> the 100 Ohm termination resistor in the chip to be enabled.
>
> Add support to setting the HS_EN bit when the optional property
> "nxp,enable-termination-resistor" is present.
>
> The motivation for introducing this property was a custom i.MX8MP board
> that was showing visual artifacts. After enabling the 100 Ohm termination
> resistor the LVDS signal quality improved causing the artifacts to
> disappear.
>
> Signed-off-by: Fabio Estevam <[email protected]>
> ---
> Changes since v1:
> - Change the property name to nxp,enable-termination-resistor (Frank).
>
> drivers/gpu/drm/bridge/fsl-ldb.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c
> b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 5c3cf37200bc..b4b9130de32c 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -92,6 +92,7 @@ struct fsl_ldb {
> const struct fsl_ldb_devdata *devdata;
> bool ch0_enabled;
> bool ch1_enabled;
> + bool termination_resistor;
Nit: To make this flag a bit more readable, I think it'd be better to
rename it to something like termination_resistor_en or use_termination_resistor.
> };
>
> static bool fsl_ldb_is_dual(const struct fsl_ldb *fsl_ldb)
> @@ -212,6 +213,10 @@ static void fsl_ldb_atomic_enable(struct drm_bridge
> *bridge,
> /* Program LVDS_CTRL */
> reg = LVDS_CTRL_CC_ADJ(2) | LVDS_CTRL_PRE_EMPH_EN |
> LVDS_CTRL_PRE_EMPH_ADJ(3) | LVDS_CTRL_VBG_EN;
> +
> + reg &= ~LVDS_CTRL_HS_EN;
This is redundant since reg is set without the LVDS_CTRL_HS_EN bit just
beforehand.
> + if (fsl_ldb->termination_resistor)
> + reg |= LVDS_CTRL_HS_EN;
> regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);
>
> /* Wait for VBG to stabilize. */
> @@ -292,6 +297,7 @@ static const struct drm_bridge_funcs funcs = {
> static int fsl_ldb_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
'dev->of_node' is already directly used by function calls in fsl_ldb_probe(),
so I'd suggest to keep using it, instead of introducing 'np'.
> struct device_node *panel_node;
> struct device_node *remote1, *remote2;
> struct drm_panel *panel;
> @@ -340,6 +346,9 @@ static int fsl_ldb_probe(struct platform_device *pdev)
> if (IS_ERR(panel))
> return PTR_ERR(panel);
>
> + if (of_property_present(np, "nxp,enable-termination-resistor"))
> + fsl_ldb->termination_resistor = true;
> +
> fsl_ldb->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> if (IS_ERR(fsl_ldb->panel_bridge))
> return PTR_ERR(fsl_ldb->panel_bridge);
--
Regards,
Liu Ying
