* [PATCH v2 0/2] Add support for Renesas RZ/G3L LVDS encoder
@ 2026-05-24 19:44 Biju
2026-05-24 19:44 ` [PATCH v2 1/2] dt-bindings: display: bridge: Document " Biju
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Biju @ 2026-05-24 19:44 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, David Airlie,
Simona Vetter, Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Luca Ceresoli, Tommaso Merciai, dri-devel, devicetree,
linux-kernel, linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das
From: Biju Das <biju.das.jz@bp.renesas.com>
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.
v2->v3:
* Dropped patch#1 as it is accepted.
* 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:
* Collected the tags for binding patches.
* 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}().
Biju Das (2):
dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder
.../bridge/renesas,r9a08g046-lvds.yaml | 128 ++++++++
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 ++
5 files changed, 445 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
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
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
2026-05-24 19:44 [PATCH v2 0/2] Add support for Renesas RZ/G3L LVDS encoder Biju
@ 2026-05-24 19:44 ` Biju
2026-05-25 6:56 ` Claude review: " Claude Code Review Bot
2026-05-24 19:44 ` [PATCH v2 2/2] drm: renesas: rz-du: Add support for " Biju
2026-05-25 6:56 ` Claude review: Add support for Renesas " Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Biju @ 2026-05-24 19:44 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Luca Ceresoli, Tommaso Merciai, dri-devel, devicetree,
linux-kernel, linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das,
Conor Dooley
From: Biju Das <biju.das.jz@bp.renesas.com>
Document the LVDS encoder IP found on the RZ/G3L SoC. It supports
single-link mode. LVDS and the DSI interface share a peripheral clock and
the MIPI_DSI_PRESET_N reset signal. However, the LVDS module cannot be
used at the same time as MIPI-DSI.
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
* No change.
v1->v2:
* Collected tag.
---
.../bridge/renesas,r9a08g046-lvds.yaml | 128 ++++++++++++++++++
1 file changed, 128 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
new file mode 100644
index 000000000000..b1f6d020ae7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/renesas,r9a08g046-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G3L LVDS Encoder
+
+maintainers:
+ - Biju Das <biju.das.jz@bp.renesas.com>
+ - Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
+
+description: |
+ This binding describe the LVDS encoder embedded in the Renesas RZ/G3L
+ SoC. The encoder can operate in LVDS Single-link mode with 4 lanes
+ (Data) + 1 lane (Clock).
+
+properties:
+ compatible:
+ const: renesas,r9a08g046-lvds
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Peripheral clock
+ - description: PHY clock
+ - description: Dot clock
+
+ clock-names:
+ items:
+ - const: pclk
+ - const: phyclk
+ - const: dotclk
+
+ resets:
+ items:
+ - description: LVDS_RESET_N
+ - description: MIPI_DSI_PRESET_N
+ - description: MIPI_DSI_CMN_RSTB
+ - description: MIPI_DSI_ARESET_N
+
+ reset-names:
+ items:
+ - const: lvdrst
+ - const: prst
+ - const: rst
+ - const: arst
+
+ power-domains:
+ maxItems: 1
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Input channel, directly connected to the Display Unit.
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: |
+ Output channel, directly connected to the LVDS panel or bridge.
+
+ required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - power-domains
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/renesas,r9a08g046-cpg.h>
+
+ lvds-cmn@108a0000 {
+ compatible = "renesas,r9a08g046-lvds-cmn",
+ "simple-mfd", "syscon";
+ reg = <0x108a0000 0x10000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ lvds0: lvds@10 {
+ compatible = "renesas,r9a08g046-lvds";
+ reg = <0x10 0x8>;
+ clocks = <&cpg CPG_MOD R9A08G046_MIPI_DSI_PCLK>,
+ <&cpg CPG_MOD R9A08G046_LVDS_PLLCLK>,
+ <&cpg CPG_MOD R9A08G046_LVDS_CLK_DOT0>;
+ clock-names = "pclk", "phyclk", "dotclk";
+ resets = <&cpg R9A08G046_LVDS_RESET_N>,
+ <&cpg R9A08G046_MIPI_DSI_PRESET_N>,
+ <&cpg R9A08G046_MIPI_DSI_CMN_RSTB>,
+ <&cpg R9A08G046_MIPI_DSI_ARESET_N>;
+ reset-names = "lvdrst", "prst", "rst", "arst";
+ power-domains = <&cpg>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ lvds0_in: endpoint {
+ remote-endpoint = <&du_out_lvds0>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ lvds0_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+ };
+ };
+...
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder
2026-05-24 19:44 [PATCH v2 0/2] Add support for Renesas RZ/G3L LVDS encoder Biju
2026-05-24 19:44 ` [PATCH v2 1/2] dt-bindings: display: bridge: Document " Biju
@ 2026-05-24 19:44 ` Biju
2026-05-25 6:56 ` Claude review: " Claude Code Review Bot
2026-05-25 6:56 ` Claude review: Add support for Renesas " Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Biju @ 2026-05-24 19:44 UTC (permalink / raw)
To: Biju Das, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Philipp Zabel, Geert Uytterhoeven,
Magnus Damm
Cc: linux-kernel, dri-devel, linux-renesas-soc, Prabhakar Mahadev Lad,
Biju Das, Tommaso Merciai
From: Biju Das <biju.das.jz@bp.renesas.com>
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@bp.renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
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 <biju.das.jz@bp.renesas.com>");
+MODULE_AUTHOR("Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>");
+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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Claude review: Add support for Renesas RZ/G3L LVDS encoder
2026-05-24 19:44 [PATCH v2 0/2] Add support for Renesas RZ/G3L LVDS encoder Biju
2026-05-24 19:44 ` [PATCH v2 1/2] dt-bindings: display: bridge: Document " Biju
2026-05-24 19:44 ` [PATCH v2 2/2] drm: renesas: rz-du: Add support for " Biju
@ 2026-05-25 6:56 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 6:56 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Add support for Renesas RZ/G3L LVDS encoder
Author: Biju <biju.das.au@gmail.com>
Patches: 3
Reviewed: 2026-05-25T16:56:33.740823
---
This is a 2-patch series adding support for the Renesas RZ/G3L LVDS encoder: a DT binding document (patch 1) and the driver itself (patch 2). The series is well-structured and follows patterns established by the existing `rzg2l_mipi_dsi.c` sibling driver. The code is generally clean and the binding document is complete.
However, there are several issues ranging from minor to moderate:
1. **Unused includes** in the driver (`of_device.h`, `of_graph.h`, several others).
2. **`rst` and `arst` reset controls are acquired but never stored** — they're asserted in probe and then lost, meaning they can never be deasserted on remove or error paths.
3. **`pm_runtime_get_sync` error path leaks the PM reference count** — on failure, `pm_runtime_put` is never called.
4. **Register header style inconsistency** — mixes `BIT()` macro with raw `(1 << n)` shifts.
5. **Cover letter says "v2->v3" but subject says "[PATCH v2]"** — version numbering is inconsistent.
The DT binding (patch 1) looks correct and already has an Acked-by from Conor Dooley.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
2026-05-24 19:44 ` [PATCH v2 1/2] dt-bindings: display: bridge: Document " Biju
@ 2026-05-25 6:56 ` Claude Code Review Bot
0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 6:56 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This patch adds the YAML DT binding for `renesas,r9a08g046-lvds`. It is well-written and already carries an Acked-by.
**Minor nits:**
1. **Description typo**: Line 803 says `"This binding describe"` — should be `"This binding describes"`.
2. **Reset ordering**: The binding defines resets as `lvdrst, prst, rst, arst`, which is fine, but the driver acquires `rst` and `arst` as `_optional_exclusive` while `prst` and `lvdrst` are `_shared`. It would be helpful to document in the binding description which resets are shared with the DSI module, since the cover letter text explains this constraint but the binding doesn't.
3. **Example node naming**: The example uses `lvds-cmn@108a0000` as the parent syscon MFD node and `lvds@10` as the child. This looks correct for a syscon child node with relative register offsets.
Overall this patch looks good. No blocking issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder
2026-05-24 19:44 ` [PATCH v2 2/2] drm: renesas: rz-du: Add support for " Biju
@ 2026-05-25 6:56 ` Claude Code Review Bot
0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 6:56 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Issues:**
**(1) `rst` and `arst` reset controls are fire-and-forget (medium severity)**
In `rzg3l_lvds_probe()`, the `rstc` and `arstc` resets are acquired, asserted, and then the local variables go out of scope. They are never stored in `struct rzg3l_lvds`, so they can never be deasserted:
```c
struct reset_control *rstc, *arstc;
...
rstc = devm_reset_control_get_optional_exclusive(dev, "rst");
...
arstc = devm_reset_control_get_optional_exclusive(dev, "arst");
...
ret = reset_control_assert(rstc);
...
ret = reset_control_assert(arstc);
```
Per the cover letter, these resets "must be asserted before using the LVDS module" and should stay asserted. If this is intentional (assert once, never deassert), it works because `devm_*` will release the reset control on driver remove, but a comment would clarify the intent. Additionally, using `_exclusive` reset controls that are only asserted and never deasserted is unusual — if they truly must stay asserted for the lifetime of the device, consider whether `_shared` would be more appropriate if the DSI driver also needs to manipulate them.
**(2) `pm_runtime_get_sync` error path leaks PM refcount (medium severity)**
In `rzg3l_lvds_atomic_enable()`:
```c
if (WARN_ON(pm_runtime_get_sync(lvds->dev) < 0))
return;
```
`pm_runtime_get_sync()` increments the reference count even on failure. On error, you must call `pm_runtime_put_noidle()` or `pm_runtime_put()` to avoid a PM refcount leak. The matching `pm_runtime_put()` in `rzg3l_lvds_atomic_disable()` will never be called if enable bailed out. Consider:
```c
if (WARN_ON(pm_runtime_get_sync(lvds->dev) < 0)) {
pm_runtime_put_noidle(lvds->dev);
return;
}
```
Or alternatively, use `pm_runtime_resume_and_get()` which doesn't increment the refcount on failure.
**(3) Unused includes (minor)**
Several headers appear unnecessary:
- `linux/of_device.h` — nothing from this header is used (no `of_device_get_match_data()` etc.)
- `linux/of_graph.h` — no `of_graph_*` functions are called
- `linux/bitfield.h` — wait, `FIELD_PREP` is used, so this one is needed
- `linux/mfd/syscon.h` — `syscon_node_to_regmap()` is used, so needed
- `linux/regmap.h` — `regmap_*` functions used, needed
- `linux/media-bus-format.h` — `MEDIA_BUS_FMT_*` used, needed
- `drm/drm_panel.h` — no panel functions called directly, but `DRM_PANEL` is selected in Kconfig. This include is not needed in this source file.
- `drm/drm_probe_helper.h` — no probe helper functions called.
**(4) Register header mixes `BIT()` and raw shifts (minor style)**
In `rzg3l_lvds_regs.h`:
```c
#define LVDS_CMN_RST_PHY0_SEL (1 << 24)
#define LVDS_CMN_RST_PHY0_SEL_CH0 (1 << 24)
#define LVDS_CMN_PHY_RESET (1 << 0)
```
These should use `BIT()` for consistency with the rest of the file which uses `BIT(8)`, `BIT(9)`, `GENMASK()`:
```c
#define LVDS_0_PHY_CH_EN_BGR BIT(8)
#define LVDS_0_PHY_CH_EN_LDO BIT(9)
```
Also, `LVDS_CMN_RST_PHY0_SEL` and `LVDS_CMN_RST_PHY0_SEL_CH0` are defined to the same value `(1 << 24)` — one of them appears redundant.
**(5) `LVDS_0_PHY_CH_IO_EN` is redundant (minor)**
```c
#define LVDS_0_PHY_CH_IO_EN_MSK (0x1f)
#define LVDS_0_PHY_CH_IO_EN (LVDS_0_PHY_CH_IO_EN_MSK << 0)
```
`LVDS_0_PHY_CH_IO_EN` is just `LVDS_0_PHY_CH_IO_EN_MSK << 0`, which is the same value. Having both is confusing. This could be simplified, or at least the `<< 0` should be removed.
**(6) `select DRM_PANEL` in Kconfig may be unnecessary (minor)**
The driver doesn't use any panel API directly — it uses `devm_drm_of_get_bridge()` which handles panel wrapping via `drm_panel_bridge`. The `DRM_PANEL` symbol is typically pulled in transitively by the bridge infrastructure. Check whether `DRM_PANEL` is really needed here.
**(7) `select DRM_KMS_HELPER` — check necessity (minor)**
Similar to above, `DRM_KMS_HELPER` is often pulled in by `DRM_BRIDGE`. Verify it's needed as a direct dependency.
**(8) The `return ret;` at the end of probe could be `return 0;` (nitpick)**
```c
ret = devm_drm_bridge_add(dev, &lvds->bridge);
if (ret)
return dev_err_probe(dev, ret,
"Failed to register drm bridge\n");
return ret;
```
At this point `ret` is always 0 (the error case already returned). Using `return 0;` is clearer.
**(9) Version numbering confusion (meta)**
The cover letter changelog says "v2->v3" but the subject lines say `[PATCH v2 0/2]`. This appears to actually be v3 based on the changelog content.
**(10) Bridge attach returns 0 when there's no next_bridge (needs verification)**
```c
static int rzg3l_lvds_attach(...)
{
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);
}
```
The `next_bridge` is set from `devm_drm_of_get_bridge()` in probe, which returns an error if not found. So `next_bridge` should always be non-NULL if probe succeeded. This NULL check is harmless defensive code but shouldn't be reachable. Not a bug, but the check is dead code given the current probe logic.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-25 6:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-24 19:44 [PATCH v2 0/2] Add support for Renesas RZ/G3L LVDS encoder Biju
2026-05-24 19:44 ` [PATCH v2 1/2] dt-bindings: display: bridge: Document " Biju
2026-05-25 6:56 ` Claude review: " Claude Code Review Bot
2026-05-24 19:44 ` [PATCH v2 2/2] drm: renesas: rz-du: Add support for " Biju
2026-05-25 6:56 ` Claude review: " Claude Code Review Bot
2026-05-25 6:56 ` Claude review: Add support for Renesas " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox