dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/2] drm: panel: support the R63419 based dual-DSI video mode Display Panels
@ 2026-06-05 14:50 Neil Armstrong
  2026-06-05 14:50 ` [PATCH v7 1/2] dt-bindings: display: panel: document the Renesas " Neil Armstrong
  2026-06-05 14:50 ` [PATCH v7 2/2] drm: panel: add support for " Neil Armstrong
  0 siblings, 2 replies; 7+ messages in thread
From: Neil Armstrong @ 2026-06-05 14:50 UTC (permalink / raw)
  To: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm
  Cc: dri-devel, devicetree, linux-kernel, linux-renesas-soc,
	Dmitry Baryshkov, Neil Armstrong, Conor Dooley, KancyJoe

Add support for the Renesas 63419 based dual-DSI video mode
Display Panels found in the Ayaneo gaming handled devices.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v7:
- Reverse "on" command order
- Rebase on drm-misc-next
- Link to v6: https://patch.msgid.link/20260522-topic-sm8650-ayaneo-pocket-s2-r63419-v6-0-16edddda9951@linaro.org

Changes in v6:
- Get new dt bindings review from Conor
- Properly init dsi_ctx
- Link to v5: https://patch.msgid.link/20260521-topic-sm8650-ayaneo-pocket-s2-r63419-v5-0-dd5700299390@linaro.org

Changes in v5:
- Import panel-common-dual.yaml in bindings
- Set reg as required
- fix bindings example typo
- Add helper to switch link in order to use single dsi_ctx to properly handle errors
- Disable vdd supplies if vcc supplied fail to enable
- Precise the power off sequence is recommended by the vendor spec
- Drop passing of node to second dsi to avoid re-probing the driver twice
- Link to v4: https://patch.msgid.link/20260519-topic-sm8650-ayaneo-pocket-s2-r63419-v4-0-b8929af5e951@linaro.org

Changes in v4:
- Moved height/width in the drm_mode, duplicated modes to use drm_connector_helper_get_modes_fixed
- Create dsi_info on the stack with proper OF node and name passed
- Switched to devm_drm_panel_add/devm_mipi_dsi_attach & dropped remove
- Link to v3: https://patch.msgid.link/20260504-topic-sm8650-ayaneo-pocket-s2-r63419-v3-0-9f61cf24aebf@linaro.org

Changes in v3:
- Added DDIC compatible as fallback
- Added rotation in bindings example
- Fixed bindings subject
- Added second MODULE_AUTHOR entry and re-ordered signed-off-by order
- Link to v2: https://patch.msgid.link/20260430-topic-sm8650-ayaneo-pocket-s2-r63419-v2-0-91ac10453d0c@linaro.org

Changes in v2:
- Add missing rotation property into bindings
- Fix commit message & subject typos
- Link to v1: https://patch.msgid.link/20260428-topic-sm8650-ayaneo-pocket-s2-r63419-v1-0-981eb5ab5a51@linaro.org

---
KancyJoe (1):
      drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels

Neil Armstrong (1):
      dt-bindings: display: panel: document the Renesas R63419 based dual-DSI video mode Display Panels

 .../bindings/display/panel/renesas,r63419.yaml     |  98 ++++++
 drivers/gpu/drm/panel/Kconfig                      |  12 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-renesas-r63419.c       | 366 +++++++++++++++++++++
 4 files changed, 477 insertions(+)
---
base-commit: 640c57d6ca1346a1c2363a3f473b405af979e046
change-id: 20260428-topic-sm8650-ayaneo-pocket-s2-r63419-e72467e2db0f

Best regards,
--  
Neil Armstrong <neil.armstrong@linaro.org>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v7 1/2] dt-bindings: display: panel: document the Renesas R63419 based dual-DSI video mode Display Panels
  2026-06-05 14:50 [PATCH v7 0/2] drm: panel: support the R63419 based dual-DSI video mode Display Panels Neil Armstrong
@ 2026-06-05 14:50 ` Neil Armstrong
  2026-06-05 14:50 ` [PATCH v7 2/2] drm: panel: add support for " Neil Armstrong
  1 sibling, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2026-06-05 14:50 UTC (permalink / raw)
  To: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm
  Cc: dri-devel, devicetree, linux-kernel, linux-renesas-soc,
	Dmitry Baryshkov, Neil Armstrong, Conor Dooley

Document the Renesas R63419 based dual-DSI video mode Display Panels found
in the Ayaneo gaming handled devices.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../bindings/display/panel/renesas,r63419.yaml     | 98 ++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/renesas,r63419.yaml b/Documentation/devicetree/bindings/display/panel/renesas,r63419.yaml
new file mode 100644
index 000000000000..adfdd2c300a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/renesas,r63419.yaml
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/renesas,r63419.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R63419 based dual-DSI video mode Display Panel
+
+maintainers:
+  - Neil Armstrong <neil.armstrong@linaro.org>
+
+description:
+  The Renesas R63419 is a generic DDIC used to control dual-DSI LCD panels.
+
+allOf:
+  - $ref: panel-common-dual.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - ayaneo,wt0600-2k
+          - ayaneo,wt0630-2k
+      - const: renesas,r63419
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+  vddio-supply: true
+  vsp-supply: true
+  vsn-supply: true
+  vci-supply: true
+
+  backlight: true
+  reset-gpios: true
+  rotation: true
+  ports: true
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - vddio-supply
+  - vsp-supply
+  - vsn-supply
+  - vci-supply
+  - backlight
+  - reset-gpios
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        panel@0 {
+            compatible = "ayaneo,wt0600-2k", "renesas,r63419";
+            reg = <0>;
+
+            reset-gpios = <&gpio 176 GPIO_ACTIVE_LOW>;
+
+            vdd-supply = <&vdd_3v0_lcd>;
+            vddio-supply = <&vdd_1v8_io>;
+            vsn-supply = <&vdd_5v0_neg>;
+            vsp-supply = <&vdd_5v0_pos>;
+            vci-supply = <&vdd_3v0_vci>;
+
+            backlight = <&backlight>;
+
+            rotation = <90>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    panel_in0: endpoint {
+                        remote-endpoint = <&dsi0_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    panel_in1: endpoint {
+                        remote-endpoint = <&dsi1_out>;
+                    };
+                };
+            };
+        };
+    };
+...

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
  2026-06-05 14:50 [PATCH v7 0/2] drm: panel: support the R63419 based dual-DSI video mode Display Panels Neil Armstrong
  2026-06-05 14:50 ` [PATCH v7 1/2] dt-bindings: display: panel: document the Renesas " Neil Armstrong
@ 2026-06-05 14:50 ` Neil Armstrong
  2026-06-05 15:00   ` sashiko-bot
                     ` (2 more replies)
  1 sibling, 3 replies; 7+ messages in thread
From: Neil Armstrong @ 2026-06-05 14:50 UTC (permalink / raw)
  To: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm
  Cc: dri-devel, devicetree, linux-kernel, linux-renesas-soc,
	Dmitry Baryshkov, Neil Armstrong, KancyJoe

From: KancyJoe <kancy2333@outlook.com>

Implement support for the Renesas 63419 based dual-DSI video mode
Display Panels found in the Ayaneo gaming handled devices.

Signed-off-by: KancyJoe <kancy2333@outlook.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/panel/Kconfig                |  12 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-renesas-r63419.c | 366 +++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 7450b27622a2..7295246cfa58 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -796,6 +796,18 @@ config DRM_PANEL_RENESAS_R61307
 	  This panel controller can be found in LG Optimus Vu P895 smartphone
 	  in combination with LCD panel.
 
+config DRM_PANEL_RENESAS_R63419
+	tristate "Renesas R63419 dual-DSI video mode panels"
+	depends on OF && GPIOLIB
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for Ayaneo WT0600 and WT0630
+	  1440x2560 60Hz dual-DSI video mode display panels with Renesas
+	  R63419 IC.
+
+	  These panels are used in Ayaneo handheld gaming devices.
+
 config DRM_PANEL_RENESAS_R69328
 	tristate "Renesas R69328 720x1280 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index c2c5cf817116..be9a6f3c9743 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
 obj-$(CONFIG_DRM_PANEL_RENESAS_R61307) += panel-renesas-r61307.o
+obj-$(CONFIG_DRM_PANEL_RENESAS_R63419) += panel-renesas-r63419.o
 obj-$(CONFIG_DRM_PANEL_RENESAS_R69328) += panel-renesas-r69328.o
 obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS581VF01) += panel-samsung-ams581vf01.o
diff --git a/drivers/gpu/drm/panel/panel-renesas-r63419.c b/drivers/gpu/drm/panel/panel-renesas-r63419.c
new file mode 100644
index 000000000000..a70de95d4f21
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-renesas-r63419.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for Renesas R63419 based dual-DSI video mode panels
+ *
+ * Copyright (c) 2025, Kancy Joe <kancy2333@outlook.com>
+ * Copyright (C) 2026 Linaro Limited
+ * Author: Neil Armstrong <neil.armstrong@linaro.org>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/drm_connector.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct renesas_r63419_panel {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi[2];
+	const struct panel_desc *desc;
+
+	struct gpio_desc *reset_gpio;
+	struct regulator_bulk_data *vdd_supplies;
+	struct regulator_bulk_data *vcc_supplies;
+	enum drm_panel_orientation orientation;
+};
+
+/* VDDIO/VDD Supplies */
+static const struct regulator_bulk_data renesas_r63419_vdd_supplies[] = {
+	{ .supply = "vddio" },
+	{ .supply = "vdd" },
+};
+
+/* VSP/VSN/VCI Supplies */
+static const struct regulator_bulk_data renesas_r63419_vcc_supplies[] = {
+	{ .supply = "vsp" },
+	{ .supply = "vsn" },
+	{ .supply = "vci" },
+};
+
+struct panel_desc {
+	const struct drm_display_mode *mode;
+	unsigned int lanes;
+	unsigned long mode_flags;
+	enum mipi_dsi_pixel_format format;
+	const struct mipi_dsi_device_info dsi_info;
+};
+
+static const struct drm_display_mode wt0600_mode = {
+	/* Dual dsi */
+	.clock = 2 * (720 + 100 + 8 + 40) * (2560 + 15 + 2 + 8) * 60 / 1000,
+	.hdisplay = 2 * 720,
+	.hsync_start = 2 * (720 + 100),
+	.hsync_end = 2 * (720 + 100 + 8),
+	.htotal = 2 * (720 + 100 + 8 + 40),
+	.vdisplay = 2560,
+	.vsync_start = 2560 + 15,
+	.vsync_end = 2560 + 15 + 2,
+	.vtotal = 2560 + 15 + 2 + 8,
+	.type = DRM_MODE_TYPE_DRIVER,
+	.width_mm = 74,
+	.height_mm = 131,
+};
+
+static const struct drm_display_mode wt0630_mode = {
+	/* Dual dsi */
+	.clock = 2 * (720 + 100 + 8 + 40) * (2560 + 15 + 2 + 8) * 60 / 1000,
+	.hdisplay = 2 * 720,
+	.hsync_start = 2 * (720 + 100),
+	.hsync_end = 2 * (720 + 100 + 8),
+	.htotal = 2 * (720 + 100 + 8 + 40),
+	.vdisplay = 2560,
+	.vsync_start = 2560 + 15,
+	.vsync_end = 2560 + 15 + 2,
+	.vtotal = 2560 + 15 + 2 + 8,
+	.type = DRM_MODE_TYPE_DRIVER,
+	.width_mm = 78,
+	.height_mm = 140,
+};
+
+static struct panel_desc wt0600_desc = {
+	.lanes = 4,
+	.mode = &wt0600_mode,
+	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+		      MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM,
+	.format = MIPI_DSI_FMT_RGB888,
+};
+
+static struct panel_desc wt0630_desc = {
+	.lanes = 4,
+	.mode = &wt0630_mode,  /* wt0600 only has different screen size */
+	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+		      MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM,
+	.format = MIPI_DSI_FMT_RGB888,
+};
+
+static inline struct renesas_r63419_panel *
+to_renesas_r63419_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct renesas_r63419_panel, panel);
+}
+
+/*
+ * Helper to switch between DSI links, so we share a single dsi_ctx
+ * for both links, so in case of an error all writes & sleep for
+ * both links are ignored.
+ */
+static inline void dsi_link_switch(struct renesas_r63419_panel *ctx,
+				   struct mipi_dsi_multi_context *dsi_ctx,
+				   unsigned int link)
+{
+	dsi_ctx->dsi = ctx->dsi[link];
+}
+
+static int renesas_r63419_on(struct renesas_r63419_panel *ctx)
+{
+	struct mipi_dsi_multi_context dsi_ctx = { 0 };
+
+	/* Panel registers are loaded from DDIC Non Volatile Memory */
+
+	dsi_link_switch(ctx, &dsi_ctx, 0);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	dsi_link_switch(ctx, &dsi_ctx, 1);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 50);
+
+	dsi_link_switch(ctx, &dsi_ctx, 0);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+	dsi_link_switch(ctx, &dsi_ctx, 1);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 150);
+
+	return dsi_ctx.accum_err;
+}
+
+static int renesas_r63419_disable(struct drm_panel *panel)
+{
+	struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
+	struct mipi_dsi_multi_context dsi_ctx = { 0 };
+
+	dsi_link_switch(ctx, &dsi_ctx, 0);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	dsi_link_switch(ctx, &dsi_ctx, 1);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 50);
+
+	dsi_link_switch(ctx, &dsi_ctx, 0);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+	dsi_link_switch(ctx, &dsi_ctx, 1);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	return dsi_ctx.accum_err;
+}
+
+static int renesas_r63419_prepare(struct drm_panel *panel)
+{
+	struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
+				    ctx->vdd_supplies);
+	if (ret < 0)
+		return ret;
+
+	usleep_range(1000, 2000);
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
+				    ctx->vcc_supplies);
+	if (ret < 0) {
+		regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
+				       ctx->vdd_supplies);
+		return ret;
+	}
+
+	usleep_range(1000, 2000);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+
+	usleep_range(3000, 4000);
+
+	ret = renesas_r63419_on(ctx);
+	if (ret < 0) {
+		dev_err(panel->dev, "Failed to initialize panel: %d\n", ret);
+
+		/* Power off sequence from the r63419 datasheet */
+		regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
+				       ctx->vcc_supplies);
+		regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
+				       ctx->vdd_supplies);
+
+		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+
+		return ret;
+	}
+
+	return 0;
+}
+
+static int renesas_r63419_unprepare(struct drm_panel *panel)
+{
+	struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
+
+	/* Power off sequence from the r63419 datasheet */
+	regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies), ctx->vcc_supplies);
+	regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies), ctx->vdd_supplies);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+
+	return 0;
+}
+
+static int renesas_r63419_get_modes(struct drm_panel *panel,
+				    struct drm_connector *connector)
+{
+	struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
+	const struct drm_display_mode *mode = ctx->desc->mode;
+
+	drm_connector_set_panel_orientation(connector, ctx->orientation);
+
+	return drm_connector_helper_get_modes_fixed(connector, mode);
+}
+
+static enum drm_panel_orientation
+renesas_r63419_get_orientation(struct drm_panel *panel)
+{
+	struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
+
+	return ctx->orientation;
+}
+
+static const struct drm_panel_funcs renesas_r63419_panel_funcs = {
+	.disable = renesas_r63419_disable,
+	.prepare = renesas_r63419_prepare,
+	.unprepare = renesas_r63419_unprepare,
+	.get_modes = renesas_r63419_get_modes,
+	.get_orientation = renesas_r63419_get_orientation,
+};
+
+static int renesas_r63419_probe(struct mipi_dsi_device *dsi)
+{
+	struct mipi_dsi_device_info info = { };
+	struct device *dev = &dsi->dev;
+	struct renesas_r63419_panel *ctx;
+	struct device_node *dsi1_node;
+	struct mipi_dsi_host *dsi1_host;
+	int ret, i;
+
+	ctx = devm_drm_panel_alloc(dev, struct renesas_r63419_panel, panel,
+				   &renesas_r63419_panel_funcs, DRM_MODE_CONNECTOR_DSI);
+	if (IS_ERR(ctx))
+		return PTR_ERR(ctx);
+
+	ctx->desc = of_device_get_match_data(dev);
+	if (!ctx->desc)
+		return dev_err_probe(dev, -ENODEV,
+				     "Failed to get panel description\n");
+
+	ret = devm_regulator_bulk_get_const(&dsi->dev,
+					    ARRAY_SIZE(renesas_r63419_vdd_supplies),
+					    renesas_r63419_vdd_supplies, &ctx->vdd_supplies);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_regulator_bulk_get_const(&dsi->dev,
+					    ARRAY_SIZE(renesas_r63419_vcc_supplies),
+					    renesas_r63419_vcc_supplies, &ctx->vcc_supplies);
+	if (ret < 0)
+		return ret;
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+				     "Failed to get reset gpio\n");
+
+	/* Get second DSI host */
+	dsi1_node = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
+	if (!dsi1_node)
+		return dev_err_probe(dev, -ENODEV,
+				     "Failed to get remote node for second DSI\n");
+
+	dsi1_host = of_find_mipi_dsi_host_by_node(dsi1_node);
+	of_node_put(dsi1_node);
+	if (!dsi1_host)
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "Failed to find second DSI host\n");
+
+	/* Copy current DSI info, do not provide OF node since no driver needs to be attached */
+	strscpy(info.type, dsi->name, sizeof(info.type));
+	info.channel = dsi->channel;
+
+	/* Register the second DSI device */
+	ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &info);
+	if (IS_ERR(ctx->dsi[1]))
+		return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]),
+				     "Failed to register second DSI device\n");
+
+	ctx->dsi[0] = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	/* Get panel orientation */
+	ret = of_drm_get_panel_orientation(dev->of_node, &ctx->orientation);
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(dev, ret,
+				     "Failed to get panel orientation\n");
+
+	ctx->panel.prepare_prev_first = true;
+
+	ret = drm_panel_of_backlight(&ctx->panel);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get backlight\n");
+
+	ret = devm_drm_panel_add(dev, &ctx->panel);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add panel\n");
+
+	/* Configure and attach both DSI devices */
+	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+		ctx->dsi[i]->lanes = ctx->desc->lanes;
+		ctx->dsi[i]->format = ctx->desc->format;
+		ctx->dsi[i]->mode_flags = ctx->desc->mode_flags;
+
+		ret = devm_mipi_dsi_attach(dev, ctx->dsi[i]);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "Failed to attach DSI device %d\n", i);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id renesas_r63419_of_match[] = {
+	{
+		.compatible = "ayaneo,wt0600-2k",
+		.data = &wt0600_desc,
+	},
+	{
+		.compatible = "ayaneo,wt0630-2k",
+		.data = &wt0630_desc,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, renesas_r63419_of_match);
+
+static struct mipi_dsi_driver renesas_r63419_driver = {
+	.probe = renesas_r63419_probe,
+	.driver = {
+		.name = "panel-renesas-r63419",
+		.of_match_table = renesas_r63419_of_match,
+	},
+};
+module_mipi_dsi_driver(renesas_r63419_driver);
+
+MODULE_AUTHOR("Kancy Joe <kancy2333@outlook.com>");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
+MODULE_DESCRIPTION("DRM driver for Renesas R63419 based dual-DSI video mode panels");
+MODULE_LICENSE("GPL");

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
  2026-06-05 14:50 ` [PATCH v7 2/2] drm: panel: add support for " Neil Armstrong
@ 2026-06-05 15:00   ` sashiko-bot
  2026-06-18 12:30   ` Neil Armstrong
  2026-06-18 17:00   ` Doug Anderson
  2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-05 15:00 UTC (permalink / raw)
  To: Neil Armstrong; +Cc: devicetree, dri-devel, wsa+renesas, conor+dt, robh

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The panel is added to the DRM subsystem before its DSI links are fully configured and attached, creating a race condition.
- [High] Asymmetric panel `.prepare`/`.enable` and `.disable`/`.unprepare` callbacks.
- [High] Missing device link to the secondary DSI host allows it to be unbound while the panel is still active, leading to a use-after-free.
--

commit c609396440aafc34634a32d089165e7df2d383ce
Author: KancyJoe <kancy2333@outlook.com>

drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels

Implement support for the Renesas 63419 based dual-DSI video mode Display
Panels found in the Ayaneo gaming handled devices.

> diff --git a/drivers/gpu/drm/panel/panel-renesas-r63419.c b/drivers/gpu/drm/panel/panel-renesas-r63419.c
> new file mode 100644
> index 0000000000000..a70de95d4f215
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-renesas-r63419.c
> @@ -0,0 +1,366 @@
[ ... ]
> +static const struct drm_panel_funcs renesas_r63419_panel_funcs = {
> +	.disable = renesas_r63419_disable,
> +	.prepare = renesas_r63419_prepare,
> +	.unprepare = renesas_r63419_unprepare,
> +	.get_modes = renesas_r63419_get_modes,
> +	.get_orientation = renesas_r63419_get_orientation,
> +};

[Severity: High]
Does this create asymmetric panel callbacks by providing .disable without
an .enable? If the DRM core or a bridge driver performs a simple display cycle
by calling drm_panel_disable() followed by drm_panel_enable() without a full
unprepare/prepare cycle, the panel will be turned off in .disable but never
turned back on since .enable is NULL.

Should the DSI power-on commands in renesas_r63419_on() be moved to an .enable
callback, or alternatively, should the power-off commands be moved to .unprepare
and .disable removed entirely?

[ ... ]
> +static int renesas_r63419_probe(struct mipi_dsi_device *dsi)
> +{
[ ... ]
> +	/* Register the second DSI device */
> +	ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &info);
> +	if (IS_ERR(ctx->dsi[1]))
> +		return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]),
> +				     "Failed to register second DSI device\n");

[Severity: High]
Does this leave the secondary DSI device vulnerable to uncoordinated unbinding?
Since ctx->dsi[1] is registered as a child of dsi1_host, if dsi1_host is unbound
via sysfs or module removal, the driver core will automatically unregister and
free its children.

Without a device link to dsi1_host->dev, the panel driver is not notified of
this removal and will retain a dangling pointer to ctx->dsi[1]. If the panel is
subsequently disabled or unbound, it could dereference ctx->dsi[1] in
renesas_r63419_disable() or trigger a double-unregister in the devres cleanup
path.

[ ... ]
> +	ret = devm_drm_panel_add(dev, &ctx->panel);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add panel\n");
> +
> +	/* Configure and attach both DSI devices */
> +	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
> +		ctx->dsi[i]->lanes = ctx->desc->lanes;
> +		ctx->dsi[i]->format = ctx->desc->format;
> +		ctx->dsi[i]->mode_flags = ctx->desc->mode_flags;
> +
> +		ret = devm_mipi_dsi_attach(dev, ctx->dsi[i]);
> +		if (ret < 0)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to attach DSI device %d\n", i);
> +	}
> +
> +	return 0;
> +}

[Severity: High]
Could this expose the panel to the DRM subsystem too early? Calling
devm_drm_panel_add() before configuring and attaching the DSI devices creates
a race condition where a display controller could bind and attempt to enable
the panel immediately.

This would invoke the panel's .prepare() callback before the DSI lanes, format,
and mode flags are applied, and before the DSI hosts are actually attached,
potentially leading to DCS command transmission failures or hardware
configuration errors. Should devm_drm_panel_add() be called after the DSI
devices are successfully attached?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-topic-sm8650-ayaneo-pocket-s2-r63419-v7-0-b84b6da84293@linaro.org?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
  2026-06-05 14:50 ` [PATCH v7 2/2] drm: panel: add support for " Neil Armstrong
  2026-06-05 15:00   ` sashiko-bot
@ 2026-06-18 12:30   ` Neil Armstrong
  2026-06-18 17:00   ` Doug Anderson
  2 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2026-06-18 12:30 UTC (permalink / raw)
  To: Dmitry Baryshkov, Jessica Zhang, Douglas Anderson
  Cc: dri-devel, devicetree, linux-kernel, linux-renesas-soc, KancyJoe,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm

Hi Dmitry, Douglas, Jessica,

On 6/5/26 16:50, Neil Armstrong wrote:
> From: KancyJoe <kancy2333@outlook.com>
> 
> Implement support for the Renesas 63419 based dual-DSI video mode
> Display Panels found in the Ayaneo gaming handled devices.
> 
> Signed-off-by: KancyJoe <kancy2333@outlook.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   drivers/gpu/drm/panel/Kconfig                |  12 +
>   drivers/gpu/drm/panel/Makefile               |   1 +
>   drivers/gpu/drm/panel/panel-renesas-r63419.c | 366 +++++++++++++++++++++++++++
>   3 files changed, 379 insertions(+)
> 
<snip>

Could one of you get a look on this patch so I can reduce my backlog ?

Jessica, do you still be marked as DRM Panel reviewer ?

Thanks,
Neil

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
  2026-06-05 14:50 ` [PATCH v7 2/2] drm: panel: add support for " Neil Armstrong
  2026-06-05 15:00   ` sashiko-bot
  2026-06-18 12:30   ` Neil Armstrong
@ 2026-06-18 17:00   ` Doug Anderson
  2026-06-19  7:47     ` Neil Armstrong
  2 siblings, 1 reply; 7+ messages in thread
From: Doug Anderson @ 2026-06-18 17:00 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, devicetree, linux-kernel,
	linux-renesas-soc, Dmitry Baryshkov, KancyJoe

Hi,

On Fri, Jun 5, 2026 at 7:51 AM Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> From: KancyJoe <kancy2333@outlook.com>
>
> Implement support for the Renesas 63419 based dual-DSI video mode
> Display Panels found in the Ayaneo gaming handled devices.
>
> Signed-off-by: KancyJoe <kancy2333@outlook.com>

I notice "Kancy Joe" has a space in the source files, but not in the
signoff. I guess Signed-off-by isn't necessarily required to be real
names these days, but still seems odd...


> +/*
> + * Helper to switch between DSI links, so we share a single dsi_ctx
> + * for both links, so in case of an error all writes & sleep for
> + * both links are ignored.
> + */
> +static inline void dsi_link_switch(struct renesas_r63419_panel *ctx,
> +                                  struct mipi_dsi_multi_context *dsi_ctx,
> +                                  unsigned int link)
> +{
> +       dsi_ctx->dsi = ctx->dsi[link];
> +}
> +
> +static int renesas_r63419_on(struct renesas_r63419_panel *ctx)
> +{
> +       struct mipi_dsi_multi_context dsi_ctx = { 0 };
> +
> +       /* Panel registers are loaded from DDIC Non Volatile Memory */
> +
> +       dsi_link_switch(ctx, &dsi_ctx, 0);
> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +       dsi_link_switch(ctx, &dsi_ctx, 1);
> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);

Instead of dsi_link_switch(), can't you use the mipi_dsi_dual()
function? I think it would be:

mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi, dsi_ctx,
ctx->dsi[0], ctx->dsi[1]);


> +static int renesas_r63419_disable(struct drm_panel *panel)
> +{
> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> +       struct mipi_dsi_multi_context dsi_ctx = { 0 };
> +
> +       dsi_link_switch(ctx, &dsi_ctx, 0);
> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +       dsi_link_switch(ctx, &dsi_ctx, 1);
> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +       mipi_dsi_msleep(&dsi_ctx, 50);
> +
> +       dsi_link_switch(ctx, &dsi_ctx, 0);
> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +       dsi_link_switch(ctx, &dsi_ctx, 1);
> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +       mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +       return dsi_ctx.accum_err;

I'm not sure we've been terribly consistent, but should the above be
"return 0"? I'm not actually sure there's any benefit to a panel's
disable() function returning an error to begin with.
drm_panel_disable() doesn't return an error, so all this does is skip
setting "panel->enabled" to false and make it harder for the system to
recover.


> +static int renesas_r63419_prepare(struct drm_panel *panel)
> +{
> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> +       int ret;
> +
> +       ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
> +                                   ctx->vdd_supplies);
> +       if (ret < 0)
> +               return ret;
> +
> +       usleep_range(1000, 2000);
> +
> +       ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
> +                                   ctx->vcc_supplies);
> +       if (ret < 0) {
> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
> +                                      ctx->vdd_supplies);
> +               return ret;
> +       }
> +
> +       usleep_range(1000, 2000);
> +
> +       gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +
> +       usleep_range(3000, 4000);
> +
> +       ret = renesas_r63419_on(ctx);
> +       if (ret < 0) {
> +               dev_err(panel->dev, "Failed to initialize panel: %d\n", ret);
> +
> +               /* Power off sequence from the r63419 datasheet */
> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
> +                                      ctx->vcc_supplies);
> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
> +                                      ctx->vdd_supplies);
> +
> +               gpiod_set_value_cansleep(ctx->reset_gpio, 1);

To make de-init opposite to init, shouldn't the reset come before you
turn the regulators off? Depending on the design of the panel, I'd
imagine this could prevent back-powering some logic?

I'd also expect vdd supplies to be turned off first?


> +static int renesas_r63419_unprepare(struct drm_panel *panel)
> +{
> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> +
> +       /* Power off sequence from the r63419 datasheet */
> +       regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies), ctx->vcc_supplies);
> +       regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies), ctx->vdd_supplies);
> +
> +       gpiod_set_value_cansleep(ctx->reset_gpio, 1);

Similar: shouldn't the reset come before the regulators to make
power-off the opposite of init.


> +static int renesas_r63419_get_modes(struct drm_panel *panel,
> +                                   struct drm_connector *connector)
> +{
> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> +       const struct drm_display_mode *mode = ctx->desc->mode;
> +
> +       drm_connector_set_panel_orientation(connector, ctx->orientation);

IIRC, the above was a workaround that caused a warning splat. Is your
panel used on a system that actually needs it? Could your DRM driver
be fixed rather than persisting this hack? For context, see commit
47bef230225b ("drm/panel: panel-edp: Implement .get_orientation
callback")


> +static int renesas_r63419_probe(struct mipi_dsi_device *dsi)
> +{
> +       struct mipi_dsi_device_info info = { };
> +       struct device *dev = &dsi->dev;
> +       struct renesas_r63419_panel *ctx;
> +       struct device_node *dsi1_node;
> +       struct mipi_dsi_host *dsi1_host;
> +       int ret, i;
> +
> +       ctx = devm_drm_panel_alloc(dev, struct renesas_r63419_panel, panel,
> +                                  &renesas_r63419_panel_funcs, DRM_MODE_CONNECTOR_DSI);
> +       if (IS_ERR(ctx))
> +               return PTR_ERR(ctx);
> +
> +       ctx->desc = of_device_get_match_data(dev);
> +       if (!ctx->desc)
> +               return dev_err_probe(dev, -ENODEV,
> +                                    "Failed to get panel description\n");
> +
> +       ret = devm_regulator_bulk_get_const(&dsi->dev,
> +                                           ARRAY_SIZE(renesas_r63419_vdd_supplies),
> +                                           renesas_r63419_vdd_supplies, &ctx->vdd_supplies);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = devm_regulator_bulk_get_const(&dsi->dev,
> +                                           ARRAY_SIZE(renesas_r63419_vcc_supplies),
> +                                           renesas_r63419_vcc_supplies, &ctx->vcc_supplies);
> +       if (ret < 0)
> +               return ret;

It seems like both sets of supplies are always enabled / disabled
together with no delay between them. Do you truly need two lists, or
can this be combined to one list of regulators. That would simplify a
bunch of logic.


> +       ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(ctx->reset_gpio))
> +               return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +                                    "Failed to get reset gpio\n");
> +
> +       /* Get second DSI host */
> +       dsi1_node = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
> +       if (!dsi1_node)
> +               return dev_err_probe(dev, -ENODEV,
> +                                    "Failed to get remote node for second DSI\n");
> +
> +       dsi1_host = of_find_mipi_dsi_host_by_node(dsi1_node);
> +       of_node_put(dsi1_node);
> +       if (!dsi1_host)
> +               return dev_err_probe(dev, -EPROBE_DEFER,
> +                                    "Failed to find second DSI host\n");
> +
> +       /* Copy current DSI info, do not provide OF node since no driver needs to be attached */
> +       strscpy(info.type, dsi->name, sizeof(info.type));

Can't you use the two-argument form of strscpy()?

FWIW, I also notice that the Sashiko AI bot had some comments. Did you
already look all of those over and decide they don't need fixing? I
have a vague recollection that there's no need to worry about someone
calling disable() and then enable() without going through the
unprepare() / prepare(). If my memory is correct, I guess that would
be nice to document... I didn't analyze some of the other claims that
the AI bot had.


-Doug

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
  2026-06-18 17:00   ` Doug Anderson
@ 2026-06-19  7:47     ` Neil Armstrong
  0 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2026-06-19  7:47 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, devicetree, linux-kernel,
	linux-renesas-soc, Dmitry Baryshkov, KancyJoe

On 6/18/26 19:00, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jun 5, 2026 at 7:51 AM Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> From: KancyJoe <kancy2333@outlook.com>
>>
>> Implement support for the Renesas 63419 based dual-DSI video mode
>> Display Panels found in the Ayaneo gaming handled devices.
>>
>> Signed-off-by: KancyJoe <kancy2333@outlook.com>
> 
> I notice "Kancy Joe" has a space in the source files, but not in the
> signoff. I guess Signed-off-by isn't necessarily required to be real
> names these days, but still seems odd...

Yeah I kept all the signoff and names as in the the source files,
it wasn't an issue for other patches, so I left it....

> 
> 
>> +/*
>> + * Helper to switch between DSI links, so we share a single dsi_ctx
>> + * for both links, so in case of an error all writes & sleep for
>> + * both links are ignored.
>> + */
>> +static inline void dsi_link_switch(struct renesas_r63419_panel *ctx,
>> +                                  struct mipi_dsi_multi_context *dsi_ctx,
>> +                                  unsigned int link)
>> +{
>> +       dsi_ctx->dsi = ctx->dsi[link];
>> +}
>> +
>> +static int renesas_r63419_on(struct renesas_r63419_panel *ctx)
>> +{
>> +       struct mipi_dsi_multi_context dsi_ctx = { 0 };
>> +
>> +       /* Panel registers are loaded from DDIC Non Volatile Memory */
>> +
>> +       dsi_link_switch(ctx, &dsi_ctx, 0);
>> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> +       dsi_link_switch(ctx, &dsi_ctx, 1);
>> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> 
> Instead of dsi_link_switch(), can't you use the mipi_dsi_dual()
> function? I think it would be:
> 
> mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi, dsi_ctx,
> ctx->dsi[0], ctx->dsi[1]);

Right, indeed will switch

> 
> 
>> +static int renesas_r63419_disable(struct drm_panel *panel)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +       struct mipi_dsi_multi_context dsi_ctx = { 0 };
>> +
>> +       dsi_link_switch(ctx, &dsi_ctx, 0);
>> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>> +       dsi_link_switch(ctx, &dsi_ctx, 1);
>> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>> +       mipi_dsi_msleep(&dsi_ctx, 50);
>> +
>> +       dsi_link_switch(ctx, &dsi_ctx, 0);
>> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>> +       dsi_link_switch(ctx, &dsi_ctx, 1);
>> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>> +       mipi_dsi_msleep(&dsi_ctx, 120);
>> +
>> +       return dsi_ctx.accum_err;
> 
> I'm not sure we've been terribly consistent, but should the above be
> "return 0"? I'm not actually sure there's any benefit to a panel's
> disable() function returning an error to begin with.
> drm_panel_disable() doesn't return an error, so all this does is skip
> setting "panel->enabled" to false and make it harder for the system to
> recover.

Yep you're right, there's no sense to return the error here.

> 
> 
>> +static int renesas_r63419_prepare(struct drm_panel *panel)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +       int ret;
>> +
>> +       ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                   ctx->vdd_supplies);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       usleep_range(1000, 2000);
>> +
>> +       ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
>> +                                   ctx->vcc_supplies);
>> +       if (ret < 0) {
>> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                      ctx->vdd_supplies);
>> +               return ret;
>> +       }
>> +
>> +       usleep_range(1000, 2000);
>> +
>> +       gpiod_set_value_cansleep(ctx->reset_gpio, 0);
>> +
>> +       usleep_range(3000, 4000);
>> +
>> +       ret = renesas_r63419_on(ctx);
>> +       if (ret < 0) {
>> +               dev_err(panel->dev, "Failed to initialize panel: %d\n", ret);
>> +
>> +               /* Power off sequence from the r63419 datasheet */
>> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
>> +                                      ctx->vcc_supplies);
>> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                      ctx->vdd_supplies);
>> +
>> +               gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> 
> To make de-init opposite to init, shouldn't the reset come before you
> turn the regulators off? Depending on the design of the panel, I'd
> imagine this could prevent back-powering some logic?
> 
> I'd also expect vdd supplies to be turned off first?

I did follow the DDIC spec here, and yeah Sashiko already
pointed it to me but reset needs to be switched to low _after_
the vdd supplies goes low. Perhaps indeed the vcc one should be
turned off after the reset. I'll do that.

> 
> 
>> +static int renesas_r63419_unprepare(struct drm_panel *panel)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +
>> +       /* Power off sequence from the r63419 datasheet */
>> +       regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies), ctx->vcc_supplies);
>> +       regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies), ctx->vdd_supplies);
>> +
>> +       gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> 
> Similar: shouldn't the reset come before the regulators to make
> power-off the opposite of init.

Yep I'll move vcc disable after the reset to match the spec more closely.

> 
> 
>> +static int renesas_r63419_get_modes(struct drm_panel *panel,
>> +                                   struct drm_connector *connector)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +       const struct drm_display_mode *mode = ctx->desc->mode;
>> +
>> +       drm_connector_set_panel_orientation(connector, ctx->orientation);
> 
> IIRC, the above was a workaround that caused a warning splat. Is your
> panel used on a system that actually needs it? Could your DRM driver
> be fixed rather than persisting this hack? For context, see commit
> 47bef230225b ("drm/panel: panel-edp: Implement .get_orientation
> callback")

No need, I'll drop it

> 
> 
>> +static int renesas_r63419_probe(struct mipi_dsi_device *dsi)
>> +{
>> +       struct mipi_dsi_device_info info = { };
>> +       struct device *dev = &dsi->dev;
>> +       struct renesas_r63419_panel *ctx;
>> +       struct device_node *dsi1_node;
>> +       struct mipi_dsi_host *dsi1_host;
>> +       int ret, i;
>> +
>> +       ctx = devm_drm_panel_alloc(dev, struct renesas_r63419_panel, panel,
>> +                                  &renesas_r63419_panel_funcs, DRM_MODE_CONNECTOR_DSI);
>> +       if (IS_ERR(ctx))
>> +               return PTR_ERR(ctx);
>> +
>> +       ctx->desc = of_device_get_match_data(dev);
>> +       if (!ctx->desc)
>> +               return dev_err_probe(dev, -ENODEV,
>> +                                    "Failed to get panel description\n");
>> +
>> +       ret = devm_regulator_bulk_get_const(&dsi->dev,
>> +                                           ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                           renesas_r63419_vdd_supplies, &ctx->vdd_supplies);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = devm_regulator_bulk_get_const(&dsi->dev,
>> +                                           ARRAY_SIZE(renesas_r63419_vcc_supplies),
>> +                                           renesas_r63419_vcc_supplies, &ctx->vcc_supplies);
>> +       if (ret < 0)
>> +               return ret;
> 
> It seems like both sets of supplies are always enabled / disabled
> together with no delay between them. Do you truly need two lists, or
> can this be combined to one list of regulators. That would simplify a
> bunch of logic.

You need a delay between enabling vcc and vdd according to the spec,
this is why I did a split.

> 
> 
>> +       ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>> +       if (IS_ERR(ctx->reset_gpio))
>> +               return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
>> +                                    "Failed to get reset gpio\n");
>> +
>> +       /* Get second DSI host */
>> +       dsi1_node = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
>> +       if (!dsi1_node)
>> +               return dev_err_probe(dev, -ENODEV,
>> +                                    "Failed to get remote node for second DSI\n");
>> +
>> +       dsi1_host = of_find_mipi_dsi_host_by_node(dsi1_node);
>> +       of_node_put(dsi1_node);
>> +       if (!dsi1_host)
>> +               return dev_err_probe(dev, -EPROBE_DEFER,
>> +                                    "Failed to find second DSI host\n");
>> +
>> +       /* Copy current DSI info, do not provide OF node since no driver needs to be attached */
>> +       strscpy(info.type, dsi->name, sizeof(info.type));
> 
> Can't you use the two-argument form of strscpy()?

Yeah sure

> 
> FWIW, I also notice that the Sashiko AI bot had some comments. Did you
> already look all of those over and decide they don't need fixing? I
> have a vague recollection that there's no need to worry about someone
> calling disable() and then enable() without going through the
> unprepare() / prepare(). If my memory is correct, I guess that would
> be nice to document... I didn't analyze some of the other claims that
> the AI bot had.

Yep I fixed the real issues, the remaining issues are about the init
sequence and some impossible init sequence between drm and dsi.

Thanks,
Neil

> 
> 
> -Doug


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-19  7:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05 14:50 [PATCH v7 0/2] drm: panel: support the R63419 based dual-DSI video mode Display Panels Neil Armstrong
2026-06-05 14:50 ` [PATCH v7 1/2] dt-bindings: display: panel: document the Renesas " Neil Armstrong
2026-06-05 14:50 ` [PATCH v7 2/2] drm: panel: add support for " Neil Armstrong
2026-06-05 15:00   ` sashiko-bot
2026-06-18 12:30   ` Neil Armstrong
2026-06-18 17:00   ` Doug Anderson
2026-06-19  7:47     ` Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox