Hi Tommaso, Thanks for the feedback.
> -----Original Message----- > From: Tommaso Merciai <[email protected]> > Sent: 26 May 2026 08:04 > Subject: Re: [PATCH v2 2/2] drm: renesas: rz-du: Add support for RZ/G3L LVDS > encoder > > Hi Biju, > Thanks for your patch. > > On Sun, May 24, 2026 at 08:44:51PM +0100, Biju wrote: > > From: Biju Das <[email protected]> > > > > Add support for the RZ/G3L LVDS encoder driver. It operates in > > single-link mode with 4 lanes (Data) + 1 lane (Clock) and supports > > pixel clock rates from 25 to 87 MHz. The LVDS module cannot be used at > > the same time as MIPI-DSI. However, LVDS and the DSI interface share a > > peripheral clock and the MIPI_DSI_PRESET_N reset signal. Also, the > > MIPI_DSI_CMN_RSTB and MIPI_DSI_ARESET_N reset signals must be asserted > > before using the LVDS module. > > > > I thinks this should be v3 instead of v2. Oops, you are correct. I will send this series as v3. Cheers, Biju > Apart from that patch LGTM. > > Tested on RZ/G3E LVDS ch0. > > Tested-by: Tommaso Merciai <[email protected]> > Reviewed-by: Tommaso Merciai <[email protected]> > > Kind Regards, > Tommaso > > > Signed-off-by: Tommaso Merciai <[email protected]> > > Signed-off-by: Biju Das <[email protected]> > > --- > > v2->v3: > > * Replace drm_atomic_state with drm_atomic_commit in > > rzg3l_lvds_atomic_{en,dis}able(). > > * Drop local variable ret and dev_err() messages in > > rzg3l_lvds_atomic_enable(); use WARN_ON() instead to > > capture unexpected failures since atomic_enable should not fail. > > * Drop local variable next_bridge from rzg3l_lvds_probe(). > > v1->v2: > > * Dropped unused function rzg3l_lvds_is_connected() and removed the > > corresponding header file rzg3l_lvds.h > > * Dropped next_bridge from struct rzg3l_lvds instead using bridge's > > next_bridge. > > * Replaced pm_runtime_resume_and_get()->pm_runtime_get_sync() as > > atomic_enable doesn't fail and for each enable there always will be an > > atomic_disable() call. > > * Started using DEFINE_RUNTIME_DEV_PM_OPS for PM callback. > > * Replaced rzg3l_lvds_parse_dt() with devm_drm_of_get_bridge() in > > probe() > > * Started using reset_control_bulk_*() in rzg3l_lvds_pm_runtime_{suspend, > > resume}() > > --- > > drivers/gpu/drm/renesas/rz-du/Kconfig | 13 + > > drivers/gpu/drm/renesas/rz-du/Makefile | 1 + > > drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c | 277 ++++++++++++++++++ > > .../gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h | 26 ++ > > 4 files changed, 317 insertions(+) > > create mode 100644 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c > > create mode 100644 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h > > > > diff --git a/drivers/gpu/drm/renesas/rz-du/Kconfig > > b/drivers/gpu/drm/renesas/rz-du/Kconfig > > index 7f2ef7137ae5..cbfc7b6bccb8 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/Kconfig > > +++ b/drivers/gpu/drm/renesas/rz-du/Kconfig > > @@ -26,3 +26,16 @@ config DRM_RZG2L_MIPI_DSI > > def_tristate DRM_RZG2L_DU > > depends on DRM_RZG2L_USE_MIPI_DSI > > select DRM_MIPI_DSI > > + > > +config DRM_RZG3L_USE_LVDS > > + bool "RZ/G3L DU LVDS Encoder Support" > > + depends on DRM_BRIDGE && OF > > + default DRM_RZG2L_DU > > + help > > + Enable support for the RZ/G3L Display Unit embedded LVDS encoders. > > + > > +config DRM_RZG3L_LVDS > > + def_tristate DRM_RZG2L_DU > > + depends on DRM_RZG3L_USE_LVDS > > + select DRM_KMS_HELPER > > + select DRM_PANEL > > diff --git a/drivers/gpu/drm/renesas/rz-du/Makefile > > b/drivers/gpu/drm/renesas/rz-du/Makefile > > index 2987900ea6b6..46decb7ac4f1 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/Makefile > > +++ b/drivers/gpu/drm/renesas/rz-du/Makefile > > @@ -8,3 +8,4 @@ rzg2l-du-drm-$(CONFIG_VIDEO_RENESAS_VSP1) += > > rzg2l_du_vsp.o > > obj-$(CONFIG_DRM_RZG2L_DU) += rzg2l-du-drm.o > > > > obj-$(CONFIG_DRM_RZG2L_MIPI_DSI) += rzg2l_mipi_dsi.o > > +obj-$(CONFIG_DRM_RZG3L_LVDS) += rzg3l_lvds.o > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c > > b/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c > > new file mode 100644 > > index 000000000000..a51c3e5a2efe > > --- /dev/null > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c > > @@ -0,0 +1,277 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * RZ/G3L LVDS Encoder Driver > > + * > > + * Copyright (C) 2026 Renesas Electronics Corporation */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/io.h> > > +#include <linux/media-bus-format.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/of_graph.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/regmap.h> > > +#include <linux/reset.h> > > + > > +#include <drm/drm_atomic.h> > > +#include <drm/drm_atomic_helper.h> > > +#include <drm/drm_bridge.h> > > +#include <drm/drm_of.h> > > +#include <drm/drm_panel.h> > > +#include <drm/drm_probe_helper.h> > > + > > +#include "rzg3l_lvds_regs.h" > > + > > +enum rzg3l_lvds_mode { > > + RZG3L_LVDS_MODE_JEIDA = 0, > > + RZG3L_LVDS_MODE_JEIDA_MIRROR = 1, > > + RZG3L_LVDS_MODE_MODE2 = 2, > > + RZG3L_LVDS_MODE_MODE2_MIRROR = 3, > > + RZG3L_LVDS_MODE_VESA = 4, > > + RZG3L_LVDS_MODE_VESA_MIRROR = 5, > > + RZG3L_LVDS_MODE_MODE6 = 6, > > + RZG3L_LVDS_MODE_MODE6_MIRROR = 7, > > +}; > > + > > +struct rzg3l_lvds { > > + struct device *dev; > > + struct reset_control *prstc; > > + struct reset_control *lvd_rstc; > > + struct regmap *regmap; > > + struct drm_bridge bridge; > > +}; > > + > > +#define bridge_to_rzg3l_lvds(b) \ > > + container_of(b, struct rzg3l_lvds, bridge) > > + > > +/* > > +--------------------------------------------------------------------- > > +-------- > > + * Bridge > > + */ > > + > > +static void rzg3l_lvds_atomic_enable(struct drm_bridge *bridge, > > + struct drm_atomic_commit *state) { > > + struct rzg3l_lvds *lvds = bridge_to_rzg3l_lvds(bridge); > > + const struct drm_bridge_state *bridge_state; > > + u32 fmt; > > + > > + /* Get the LVDS format from the bridge state. */ > > + bridge_state = drm_atomic_get_new_bridge_state(state, bridge); > > + if (WARN_ON(!bridge_state)) > > + return; > > + > > + switch (bridge_state->output_bus_cfg.format) { > > + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA: > > + fmt = RZG3L_LVDS_MODE_JEIDA; > > + break; > > + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG: > > + fmt = RZG3L_LVDS_MODE_VESA; > > + break; > > + default: > > + fmt = RZG3L_LVDS_MODE_VESA; > > + dev_warn(lvds->dev, "Unsupported bus fmt 0x%04x\n", > > + bridge_state->output_bus_cfg.format); > > + break; > > + } > > + > > + if (WARN_ON(pm_runtime_get_sync(lvds->dev) < 0)) > > + return; > > + > > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET, > > + LVDS_0_PHY_CH_EN_BGR, LVDS_0_PHY_CH_EN_BGR); > > + fsleep(20); > > + > > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET, > > + LVDS_0_PHY_CH_EN_LDO, LVDS_0_PHY_CH_EN_LDO); > > + fsleep(10); > > + > > + regmap_write(lvds->regmap, LVDS_CMN, LVDS_CMN_RST_PHY0_SEL); > > + regmap_update_bits(lvds->regmap, LVDS_0_CTL_OFFSET, > > + LVDS_0_CTL_FMT_SEL_MSK, > > + FIELD_PREP(LVDS_0_CTL_FMT_SEL_MSK, fmt)); > > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET, > > + LVDS_0_PHY_CH_IO_EN_MSK, LVDS_0_PHY_CH_IO_EN); > > + regmap_write(lvds->regmap, LVDS_CMN, > > + LVDS_CMN_RST_PHY0_SEL | LVDS_CMN_PHY_RESET); > > + fsleep(100); > > +} > > + > > +static void rzg3l_lvds_atomic_disable(struct drm_bridge *bridge, > > + struct drm_atomic_commit *state) { > > + struct rzg3l_lvds *lvds = bridge_to_rzg3l_lvds(bridge); > > + > > + regmap_update_bits(lvds->regmap, LVDS_CMN, LVDS_CMN_PHY_RESET, 0); > > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET, > > + LVDS_0_PHY_CH_IO_EN_MSK, 0); > > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET, > > + LVDS_0_PHY_CH_EN_LDO, 0); > > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET, > > + LVDS_0_PHY_CH_EN_BGR, 0); > > + > > + pm_runtime_put(lvds->dev); > > +} > > + > > +static int rzg3l_lvds_attach(struct drm_bridge *bridge, > > + struct drm_encoder *encoder, > > + enum drm_bridge_attach_flags flags) { > > + struct rzg3l_lvds *lvds = bridge_to_rzg3l_lvds(bridge); > > + > > + if (!lvds->bridge.next_bridge) > > + return 0; > > + > > + return drm_bridge_attach(encoder, lvds->bridge.next_bridge, bridge, > > +flags); } > > + > > +static enum drm_mode_status > > +rzg3l_lvds_bridge_mode_valid(struct drm_bridge *bridge, > > + const struct drm_display_info *info, > > + const struct drm_display_mode *mode) { > > + if (mode->clock > 87000) > > + return MODE_CLOCK_HIGH; > > + > > + if (mode->clock < 25000) > > + return MODE_CLOCK_LOW; > > + > > + return MODE_OK; > > +} > > + > > +static const struct drm_bridge_funcs rzg3l_lvds_bridge_ops = { > > + .attach = rzg3l_lvds_attach, > > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > > + .atomic_reset = drm_atomic_helper_bridge_reset, > > + .atomic_enable = rzg3l_lvds_atomic_enable, > > + .atomic_disable = rzg3l_lvds_atomic_disable, > > + .mode_valid = rzg3l_lvds_bridge_mode_valid, }; > > + > > +/* > > +--------------------------------------------------------------------- > > +-------- > > + * Power Management > > + */ > > + > > +static int rzg3l_lvds_pm_runtime_suspend(struct device *dev) { > > + struct rzg3l_lvds *lvds = dev_get_drvdata(dev); > > + struct reset_control_bulk_data resets[] = { > > + { .rstc = lvds->lvd_rstc }, > > + { .rstc = lvds->prstc }, > > + }; > > + > > + return reset_control_bulk_assert(ARRAY_SIZE(resets), resets); } > > + > > +static int rzg3l_lvds_pm_runtime_resume(struct device *dev) { > > + struct rzg3l_lvds *lvds = dev_get_drvdata(dev); > > + struct reset_control_bulk_data resets[] = { > > + { .rstc = lvds->lvd_rstc }, > > + { .rstc = lvds->prstc }, > > + }; > > + > > + return reset_control_bulk_deassert(ARRAY_SIZE(resets), resets); } > > + > > +static DEFINE_RUNTIME_DEV_PM_OPS(rzg3l_lvds_pm_ops, > > + rzg3l_lvds_pm_runtime_suspend, > > + rzg3l_lvds_pm_runtime_resume, NULL); > > + > > +/* > > +--------------------------------------------------------------------- > > +-------- > > + * Probe & Remove > > + */ > > + > > +static int rzg3l_lvds_probe(struct platform_device *pdev) { > > + struct reset_control *rstc, *arstc; > > + struct device *dev = &pdev->dev; > > + struct rzg3l_lvds *lvds; > > + int ret; > > + > > + lvds = devm_drm_bridge_alloc(dev, struct rzg3l_lvds, bridge, > > + &rzg3l_lvds_bridge_ops); > > + if (IS_ERR(lvds)) > > + return PTR_ERR(lvds); > > + > > + lvds->dev = dev; > > + lvds->bridge.of_node = pdev->dev.of_node; > > + > > + lvds->regmap = syscon_node_to_regmap(dev->of_node->parent); > > + if (IS_ERR(lvds->regmap)) > > + return PTR_ERR(lvds->regmap); > > + > > + rstc = devm_reset_control_get_optional_exclusive(dev, "rst"); > > + if (IS_ERR(rstc)) > > + return dev_err_probe(dev, PTR_ERR(rstc), "failed to get rst\n"); > > + > > + arstc = devm_reset_control_get_optional_exclusive(dev, "arst"); > > + if (IS_ERR(arstc)) > > + return dev_err_probe(dev, PTR_ERR(arstc), > > + "failed to get arst\n"); > > + > > + lvds->prstc = devm_reset_control_get_shared(dev, "prst"); > > + if (IS_ERR(lvds->prstc)) > > + return dev_err_probe(dev, PTR_ERR(lvds->prstc), > > + "failed to get prst\n"); > > + > > + lvds->lvd_rstc = devm_reset_control_get_shared(dev, "lvdrst"); > > + if (IS_ERR(lvds->lvd_rstc)) > > + return dev_err_probe(dev, PTR_ERR(lvds->lvd_rstc), > > + "failed to get core reset\n"); > > + > > + platform_set_drvdata(pdev, lvds); > > + ret = devm_pm_runtime_enable(dev); > > + if (ret) > > + return dev_err_probe(dev, ret, "Failed to enable Runtime PM\n"); > > + > > + lvds->bridge.next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, > > 0); > > + if (IS_ERR(lvds->bridge.next_bridge)) > > + return dev_err_probe(dev, PTR_ERR(lvds->bridge.next_bridge), > > + "failed to get next bridge\n"); > > + > > + ret = reset_control_assert(rstc); > > + if (ret < 0) > > + return ret; > > + > > + ret = reset_control_assert(arstc); > > + if (ret < 0) > > + return ret; > > + > > + ret = devm_drm_bridge_add(dev, &lvds->bridge); > > + if (ret) > > + return dev_err_probe(dev, ret, > > + "Failed to register drm bridge\n"); > > + > > + return ret; > > +} > > + > > +static const struct of_device_id rzg3l_lvds_of_table[] = { > > + { .compatible = "renesas,r9a08g046-lvds" }, > > + { /* sentinel */ } > > +}; > > + > > +MODULE_DEVICE_TABLE(of, rzg3l_lvds_of_table); > > + > > +static struct platform_driver rzg3l_lvds_platform_driver = { > > + .probe = rzg3l_lvds_probe, > > + .driver = { > > + .name = "rzg3l-lvds", > > + .pm = pm_ptr(&rzg3l_lvds_pm_ops), > > + .of_match_table = rzg3l_lvds_of_table, > > + }, > > +}; > > + > > +module_platform_driver(rzg3l_lvds_platform_driver); > > + > > +MODULE_AUTHOR("Biju Das <[email protected]>"); > > +MODULE_AUTHOR("Tommaso Merciai <[email protected]>"); > > +MODULE_DESCRIPTION("Renesas RZ/G3L LVDS Encoder Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h > > b/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h > > new file mode 100644 > > index 000000000000..281b7648f168 > > --- /dev/null > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h > > @@ -0,0 +1,26 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * RZ/G3L LVDS Interface Registers Definitions > > + * > > + * Copyright (C) 2026 Renesas Electronics Corporation > > + * > > + */ > > + > > +#ifndef __RZG3L_LVDS_REGS_H__ > > +#define __RZG3L_LVDS_REGS_H__ > > + > > +#define LVDS_CMN 0x00 > > +#define LVDS_CMN_RST_PHY0_SEL (1 << 24) > > +#define LVDS_CMN_RST_PHY0_SEL_CH0 (1 << 24) > > +#define LVDS_CMN_PHY_RESET (1 << 0) > > + > > +#define LVDS_0_PHY_OFFSET 0x10 > > +#define LVDS_0_PHY_CH_IO_EN_MSK (0x1f) > > +#define LVDS_0_PHY_CH_IO_EN (LVDS_0_PHY_CH_IO_EN_MSK << 0) > > +#define LVDS_0_PHY_CH_EN_BGR BIT(8) > > +#define LVDS_0_PHY_CH_EN_LDO BIT(9) > > + > > +#define LVDS_0_CTL_OFFSET 0x14 > > +#define LVDS_0_CTL_FMT_SEL_MSK GENMASK(23, 20) > > + > > +#endif /* __RZG3L_LVDS_REGS_H__ */ > > -- > > 2.43.0 > >
