Re: [PATCH v2 2/2] drm/panel: Add driver for EDO RM69380 OLED panel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 15/04/2024 à 18:10, David Wronek a écrit :
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.

Signed-off-by: David Wronek <david-vu3DzTD92ROXwddmVfQv5g@xxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/panel/Kconfig                 |  14 +
  drivers/gpu/drm/panel/Makefile                |   1 +
  drivers/gpu/drm/panel/panel-raydium-rm69380.c | 366 ++++++++++++++++++++++++++
  3 files changed, 381 insertions(+)


...

+static int rm69380_on(struct rm69380_panel *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi[0];
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+	if (ctx->dsi[1])
+		ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+	mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+	mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+	mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+	mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+	mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+	mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0x53, 0x28);
+	mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+	mipi_dsi_dcs_write_seq(dsi, 0x35, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0x51, 0x07, 0xff);
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+		return ret;
+	}
+	msleep(20);
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display on: %d\n", ret);
+		return ret;
+	}
+	msleep(36);

36 and 35 below are un-usual values for msleep.

Why 2 different values?
Would using a #define for this(these) value(s) make sense here?

+
+	return 0;
+}
+
+static int rm69380_off(struct rm69380_panel *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi[0];
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+	if (ctx->dsi[1])
+		ctx->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display off: %d\n", ret);
+		return ret;
+	}
+	msleep(35);

(here)

+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+		return ret;
+	}
+	msleep(20);
+
+	return 0;
+}

...

+static int rm69380_probe(struct mipi_dsi_device *dsi)
+{
+	struct mipi_dsi_host *dsi_sec_host;
+	struct rm69380_panel *ctx;
+	struct device *dev = &dsi->dev;
+	struct device_node *dsi_sec;
+	int ret, i;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->supplies[0].supply = "vddio";
+	ctx->supplies[1].supply = "avdd";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+	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-gpios\n");
+
+	dsi_sec = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
+
+	if (dsi_sec) {
+		dev_dbg(dev, "Using Dual-DSI\n");

This should be after de 'info' variable below, so...

+
+		const struct mipi_dsi_device_info info = { "RM69380", 0,
+							   dsi_sec };
+
+		dev_dbg(dev, "Found second DSI `%s`\n", dsi_sec->name);

... maybe merge the 2 messages into something like:
      dev_dbg(dev, "Using Dual-DSI: found `%s`\n", dsi_sec->name);

+
+		dsi_sec_host = of_find_mipi_dsi_host_by_node(dsi_sec);
+		of_node_put(dsi_sec);
+		if (!dsi_sec_host) {
+			return dev_err_probe(dev, -EPROBE_DEFER,
+					     "Cannot get secondary DSI host\n");
+		}
+

Nit: unneeded { }

+		ctx->dsi[1] =
+			mipi_dsi_device_register_full(dsi_sec_host, &info);
+		if (IS_ERR(ctx->dsi[1])) {
+			return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]),
+					     "Cannot get secondary DSI node\n");
+		}

Nit: unneeded { }

+
+		dev_dbg(dev, "Second DSI name `%s`\n", ctx->dsi[1]->name);
+		mipi_dsi_set_drvdata(ctx->dsi[1], ctx);
+	} else {
+		dev_dbg(dev, "Using Single-DSI\n");
+	}
+
+	ctx->dsi[0] = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	drm_panel_init(&ctx->panel, dev, &rm69380_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+	ctx->panel.prepare_prev_first = true;
+
+	ctx->panel.backlight = rm69380_create_backlight(dsi);
+	if (IS_ERR(ctx->panel.backlight))
+		return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
+				     "Failed to create backlight\n");
+
+	drm_panel_add(&ctx->panel);
+
+	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+		if (!ctx->dsi[i])
+			continue;
+
+		dev_dbg(&ctx->dsi[i]->dev, "Binding DSI %d\n", i);
+
+		ctx->dsi[i]->lanes = 4;
+		ctx->dsi[i]->format = MIPI_DSI_FMT_RGB888;
+		ctx->dsi[i]->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
+					  MIPI_DSI_CLOCK_NON_CONTINUOUS;
+
+		ret = mipi_dsi_attach(ctx->dsi[i]);
+		if (ret < 0) {
+			drm_panel_remove(&ctx->panel);
+			return dev_err_probe(dev, ret,
+					     "Failed to attach to DSI%d\n", i);

The error handling looks incomplete.
Previous mipi_dsi_attach() should be undone by a mipi_dsi_detach() as done in the remove function.

+		}
+	}
+
+	return 0;
+}

...

CJ




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux