RE: [PATCH 3/3] drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder
From: Biju Das
Date: Tue Apr 21 2026 - 12:22:16 EST
Hi Dmitry Baryshkov,
Thanks for the feedback.
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> Sent: 19 April 2026 16:59
> Subject: Re: [PATCH 3/3] drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder
>
> On Fri, Apr 17, 2026 at 06:52:30PM +0100, Biju wrote:
> > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > 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.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > 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 | 333 ++++++++++++++++++
> > drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.h | 22 ++
> > .../gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h | 26 ++
> > 5 files changed, 395 insertions(+)
> > create mode 100644 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c
> > create mode 100644 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.h
> > 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..bedeedbdfada
> > --- /dev/null
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c
> > @@ -0,0 +1,333 @@
> > +// 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.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;
> > + struct drm_bridge *next_bridge;
>
> Please use next_bridge from the drm_bridge struct.
OK.
>
> > +};
> > +
> > +#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_state *state) {
> > + struct rzg3l_lvds *lvds = bridge_to_rzg3l_lvds(bridge);
> > + const struct drm_bridge_state *bridge_state;
> > + int ret;
> > + u32 fmt;
> > +
> > + /* Get the LVDS format from the bridge state. */
> > + bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> > + if (!bridge_state) {
> > + dev_err(lvds->dev, "failed to get bridge state\n");
> > + 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;
> > + }
> > +
> > + ret = pm_runtime_resume_and_get(lvds->dev);
>
> If this fails for any reason, the atomic_disable() would still be called and it will decrement the
> counter, potentially undeflowing it.
> Consider switching to pm_runtime_get_sync(), which suits better here.
Agreed.
>
>
> > + if (ret < 0) {
> > + dev_err(lvds->dev, "pm_runtime_resume_and_get error\n");
> > + return;
> > + }
> > +
> > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET,
> > + LVDS_0_PHY_CH_EN_BGR, LVDS_0_PHY_CH_EN_BGR);
> > + usleep_range(20, 25);
> > +
> > + regmap_update_bits(lvds->regmap, LVDS_0_PHY_OFFSET,
> > + LVDS_0_PHY_CH_EN_LDO, LVDS_0_PHY_CH_EN_LDO);
> > + usleep_range(10, 15);
> > +
> > + 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);
> > + usleep_range(100, 150);
> > +}
> > +
> > +static void rzg3l_lvds_atomic_disable(struct drm_bridge *bridge,
> > + struct drm_atomic_state *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->next_bridge)
> > + return 0;
> > +
> > + return drm_bridge_attach(encoder, lvds->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;
> > +}
> > +
> > +bool rzg3l_lvds_is_connected(struct drm_bridge *bridge) {
> > + struct rzg3l_lvds *lvds = bridge_to_rzg3l_lvds(bridge);
> > +
> > + return !!lvds->next_bridge;
> > +}
> > +EXPORT_SYMBOL_GPL(rzg3l_lvds_is_connected);
>
> How is this going to be used? I don't see the user in the patch. Please drop the unused API.
OK, will drop this patch as it is not required for this platform.
>
> > +
> > +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);
> > + int ret;
> > +
> > + ret = reset_control_assert(lvds->lvd_rstc);
> > + if (ret)
> > + return ret;
> > +
> > + ret = reset_control_assert(lvds->prstc);
> > + if (ret)
> > + goto err_deassert_lvd_rstc;
> > +
> > + return 0;
> > +
> > +err_deassert_lvd_rstc:
> > + reset_control_deassert(lvds->lvd_rstc);
> > + return ret;
> > +}
> > +
> > +static int rzg3l_lvds_pm_runtime_resume(struct device *dev) {
> > + struct rzg3l_lvds *lvds = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = reset_control_deassert(lvds->prstc);
> > + if (ret)
> > + return ret;
> > +
> > + return reset_control_deassert(lvds->lvd_rstc);
> > + if (ret)
> > + goto err_assert_prstc;
> > +
> > + return 0;
> > +
> > +err_assert_prstc:
> > + reset_control_assert(lvds->prstc);
> > + return ret;
> > +}
> > +
> > +static const struct dev_pm_ops rzg3l_lvds_pm_ops = {
> > + RUNTIME_PM_OPS(rzg3l_lvds_pm_runtime_suspend,
> > + rzg3l_lvds_pm_runtime_resume, NULL)
> > + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > +pm_runtime_force_resume) };
>
> DEFINE_RUNTIME_DEV_PM_OPS()
OK. Will send v2 with the above changes.
Cheers,
Biju