[PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[Abhinav Kumar]

From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>

Add support for Truly NT35597 panel used
in MSM reference platforms.

This panel supports both single DSI and dual DSI
modes.

However, this patch series adds support only for
dual DSI mode.

Changes in v5:
- Added comments for the delays
- Fix error messages and return code
- Start using backlight_enable/disable helpers
- Start using ARRAY_SIZE everywhere
- Split the panel commands into three sets to
  remove redundant structure fields and simplify
  the DCS command sending method
- Use of_get_drm_display_mode() and simplify
  get_modes function
- Remove truly_wqxga_panel_del and do necessary
  cleanup
- Replace dev_err with DRM_DEV_ERROR

Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
---
 drivers/gpu/drm/panel/Kconfig               |   8 +
 drivers/gpu/drm/panel/Makefile              |   1 +
 drivers/gpu/drm/panel/panel-truly-nt35597.c | 680 ++++++++++++++++++++++++++++
 3 files changed, 689 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35597.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 6020c30..7ae74c2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -186,4 +186,12 @@ config DRM_PANEL_SITRONIX_ST7789V
 	  Say Y here if you want to enable support for the Sitronix
 	  ST7789V controller for 240x320 LCD panels
 
+config DRM_PANEL_TRULY_NT35597_WQXGA
+	tristate "Truly WQXGA"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	select VIDEOMODE_HELPERS
+	help
+	  Say Y here if you want to enable support for Truly NT35597 WQXGA Dual DSI
+	  Video Mode panel
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 5ccaaa9..80fd19f 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
+obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c b/drivers/gpu/drm/panel/panel-truly-nt35597.c
new file mode 100644
index 0000000..938953b
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c
@@ -0,0 +1,680 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_mipi_dsi.h>
+
+static const char * const regulator_names[] = {
+	"vdda",
+	"vdispp",
+	"vdispn",
+};
+
+static unsigned long const regulator_enable_loads[] = {
+	62000,
+	100000,
+	100000,
+};
+
+static unsigned long const regulator_disable_loads[] = {
+	80,
+	100,
+	100,
+};
+
+struct truly_wqxga {
+	struct device *dev;
+	struct drm_panel panel;
+
+	struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *mode_gpio;
+
+	struct backlight_device *backlight;
+	struct drm_display_mode dm;
+	u32 bus_flags;
+	struct mipi_dsi_device *dsi[2];
+
+	bool prepared;
+	bool enabled;
+};
+
+static inline struct truly_wqxga *panel_to_truly_wqxga(struct drm_panel *panel)
+{
+	return container_of(panel, struct truly_wqxga, panel);
+}
+
+#define MAX_LEN	5
+#define SHORT_PACKET 2
+#define LONG_PACKET 4
+
+struct cmd_set {
+	u8 commands[MAX_LEN];
+};
+
+struct cmd_set panel_cmds_set_1[] = {
+	/* CMD2_P0 */
+	{ { 0xff, 0x20 } },
+	{ { 0xfb, 0x01 } },
+	{ { 0x00, 0x01 } },
+	{ { 0x01, 0x55 } },
+	{ { 0x02, 0x45 } },
+	{ { 0x05, 0x40 } },
+	{ { 0x06, 0x19 } },
+	{ { 0x07, 0x1e } },
+	{ { 0x0b, 0x73 } },
+	{ { 0x0c, 0x73 } },
+	{ { 0x0e, 0xb0 } },
+	{ { 0x0f, 0xae } },
+	{ { 0x11, 0xb8 } },
+	{ { 0x13, 0x00 } },
+	{ { 0x58, 0x80 } },
+	{ { 0x59, 0x01 } },
+	{ { 0x5a, 0x00 } },
+	{ { 0x5b, 0x01 } },
+	{ { 0x5c, 0x80 } },
+	{ { 0x5d, 0x81 } },
+	{ { 0x5e, 0x00 } },
+	{ { 0x5f, 0x01 } },
+	{ { 0x72, 0x11 } },
+	{ { 0x68, 0x03 } },
+	/* CMD2_P4 */
+	{ { 0xFF, 0x24 } },
+	{ { 0xFB, 0x01 } },
+	{ { 0x00, 0x1C } },
+	{ { 0x01, 0x0B } },
+	{ { 0x02, 0x0C } },
+	{ { 0x03, 0x01 } },
+	{ { 0x04, 0x0F } },
+	{ { 0x05, 0x10 } },
+	{ { 0x06, 0x10 } },
+	{ { 0x07, 0x10 } },
+	{ { 0x08, 0x89 } },
+	{ { 0x09, 0x8A } },
+	{ { 0x0A, 0x13 } },
+	{ { 0x0B, 0x13 } },
+	{ { 0x0C, 0x15 } },
+	{ { 0x0D, 0x15 } },
+	{ { 0x0E, 0x17 } },
+	{ { 0x0F, 0x17 } },
+	{ { 0x10, 0x1C } },
+	{ { 0x11, 0x0B } },
+	{ { 0x12, 0x0C } },
+	{ { 0x13, 0x01 } },
+	{ { 0x14, 0x0F } },
+	{ { 0x15, 0x10 } },
+	{ { 0x16, 0x10 } },
+	{ { 0x17, 0x10 } },
+	{ { 0x18, 0x89 } },
+	{ { 0x19, 0x8A } },
+	{ { 0x1A, 0x13 } },
+	{ { 0x1B, 0x13 } },
+	{ { 0x1C, 0x15 } },
+	{ { 0x1D, 0x15 } },
+	{ { 0x1E, 0x17 } },
+	{ { 0x1F, 0x17 } },
+	/* STV */
+	{ { 0x20, 0x40 } },
+	{ { 0x21, 0x01 } },
+	{ { 0x22, 0x00 } },
+	{ { 0x23, 0x40 } },
+	{ { 0x24, 0x40 } },
+	{ { 0x25, 0x6D } },
+	{ { 0x26, 0x40 } },
+	{ { 0x27, 0x40 } },
+	/* Vend */
+	{ { 0xE0, 0x00 } },
+	{ { 0xDC, 0x21 } },
+	{ { 0xDD, 0x22 } },
+	{ { 0xDE, 0x07 } },
+	{ { 0xDF, 0x07 } },
+	{ { 0xE3, 0x6D } },
+	{ { 0xE1, 0x07 } },
+	{ { 0xE2, 0x07 } },
+	/* UD */
+	{ { 0x29, 0xD8 } },
+	{ { 0x2A, 0x2A } },
+	/* CLK */
+	{ { 0x4B, 0x03 } },
+	{ { 0x4C, 0x11 } },
+	{ { 0x4D, 0x10 } },
+	{ { 0x4E, 0x01 } },
+	{ { 0x4F, 0x01 } },
+	{ { 0x50, 0x10 } },
+	{ { 0x51, 0x00 } },
+	{ { 0x52, 0x80 } },
+	{ { 0x53, 0x00 } },
+	{ { 0x56, 0x00 } },
+	{ { 0x54, 0x07 } },
+	{ { 0x58, 0x07 } },
+	{ { 0x55, 0x25 } },
+	/* Reset XDONB */
+	{ { 0x5B, 0x43 } },
+	{ { 0x5C, 0x00 } },
+	{ { 0x5F, 0x73 } },
+	{ { 0x60, 0x73 } },
+	{ { 0x63, 0x22 } },
+	{ { 0x64, 0x00 } },
+	{ { 0x67, 0x08 } },
+	{ { 0x68, 0x04 } },
+	/* Resolution:1440x2560 */
+	{ { 0x72, 0x02 } },
+	/* mux */
+	{ { 0x7A, 0x80 } },
+	{ { 0x7B, 0x91 } },
+	{ { 0x7C, 0xD8 } },
+	{ { 0x7D, 0x60 } },
+	{ { 0x7F, 0x15 } },
+	{ { 0x75, 0x15 } },
+	/* ABOFF */
+	{ { 0xB3, 0xC0 } },
+	{ { 0xB4, 0x00 } },
+	{ { 0xB5, 0x00 } },
+	/* Source EQ */
+	{ { 0x78, 0x00 } },
+	{ { 0x79, 0x00 } },
+	{ { 0x80, 0x00 } },
+	{ { 0x83, 0x00 } },
+	/* FP BP */
+	{ { 0x93, 0x0A } },
+	{ { 0x94, 0x0A } },
+	/* Inversion Type */
+	{ { 0x8A, 0x00 } },
+	{ { 0x9B, 0xFF } },
+	/* IMGSWAP =1 @PortSwap=1 */
+	{ { 0x9D, 0xB0 } },
+	{ { 0x9F, 0x63 } },
+	{ { 0x98, 0x10 } },
+	/* FRM */
+	{ { 0xEC, 0x00 } },
+	/* CMD1 */
+	{ { 0xFF, 0x10 } },
+};
+
+struct cmd_set panel_cmds_set_2[] = {
+	/* VBP+VSA=,VFP = 10H */
+	{ { 0x3B, 0x03, 0x0A, 0x0A } },
+};
+
+struct cmd_set panel_cmds_set_3[] = {
+	/* FTE on */
+	{ { 0x35, 0x00 } },
+	/* EN_BK =1(auto black) */
+	{ { 0xE5, 0x01 } },
+	/* CMD mode(10) VDO mode(03) */
+	{ { 0xBB, 0x03 } },
+	/* Non Reload MTP */
+	{ { 0xFB, 0x01 } },
+};
+
+static int truly_dcs_write(struct drm_panel *panel, u32 command)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+		ret = mipi_dsi_dcs_write(ctx->dsi[i],
+			command, NULL, 0);
+		if (ret < 0) {
+			DRM_DEV_ERROR(ctx->dev,
+				"set_display_off cmd failed for dsi = %d\n",
+			i);
+		}
+	}
+
+	return ret;
+}
+
+static int truly_dcs_write_buf(struct drm_panel *panel,
+	u32 size, u32 num_cmds, struct cmd_set *buf)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret = 0;
+	int i, j;
+
+	for (i = 0; i < num_cmds; i++) {
+		for (j = 0; j < ARRAY_SIZE(ctx->dsi); j++) {
+			ret = mipi_dsi_dcs_write_buffer(ctx->dsi[j],
+					buf[i].commands,
+					size);
+			if (ret < 0) {
+				DRM_DEV_ERROR(ctx->dev,
+					"failed to tx cmd [%d], err: %d\n",
+					i, ret);
+				return ret;
+			}
+		}
+	}
+
+	return ret;
+}
+
+#define truly_call_write_func(ret, func, panel, command) {\
+	ret = func(panel, command);     \
+}
+
+#define truly_write_buf_func(ret, func, panel, size, cmds, buf) {\
+	ret = func(panel, size, cmds, buf); \
+}
+
+static int truly_wqxga_power_on(struct truly_wqxga *ctx)
+{
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+					regulator_enable_loads[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	/* Reset sequence of truly panel requires
+	 * the reset pin to be pulled high for 10ms
+	 * followed by a low period of 10ms and then
+	 * pulled high again
+	 */
+
+	gpiod_set_value(ctx->reset_gpio, 1);
+	msleep(20);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	msleep(20);
+	gpiod_set_value(ctx->reset_gpio, 1);
+
+	return 0;
+}
+
+static int truly_wqxga_power_off(struct truly_wqxga *ctx)
+{
+	int ret, i;
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+				regulator_disable_loads[i]);
+		if (ret)
+			return ret;
+	}
+
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static int truly_wqxga_disable(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret;
+
+	if (!ctx->enabled)
+		return 0;
+
+	if (ctx->backlight) {
+		ret = backlight_disable(ctx->backlight);
+		if (ret < 0)
+			DRM_DEV_ERROR(ctx->dev, "backlight disable failed %d\n",
+				ret);
+	}
+
+	ctx->enabled = false;
+	return 0;
+}
+
+static int truly_wqxga_unprepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret = 0;
+
+	if (!ctx->prepared)
+		return 0;
+
+	ctx->dsi[0]->mode_flags = 0;
+	ctx->dsi[1]->mode_flags = 0;
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_SET_DISPLAY_OFF);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"set_display_off cmd failed ret = %d\n",
+			ret);
+	}
+
+	/* 120ms delay required here as per DCS spec */
+	msleep(120);
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_ENTER_SLEEP_MODE);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"enter_sleep cmd failed ret = %d\n",
+			ret);
+	}
+
+	ret = truly_wqxga_power_off(ctx);
+	if (ret < 0)
+		DRM_DEV_ERROR(ctx->dev,
+			"truly_wqxga_power_off failed ret = %d\n",
+			ret);
+
+	ctx->prepared = false;
+	return ret;
+}
+
+static int truly_wqxga_prepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret;
+
+	if (ctx->prepared)
+		return 0;
+
+	ret = truly_wqxga_power_on(ctx);
+	if (ret < 0)
+		return ret;
+
+	ctx->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
+	ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	truly_write_buf_func(ret, truly_dcs_write_buf,
+		panel, SHORT_PACKET,
+		ARRAY_SIZE(panel_cmds_set_1),
+		panel_cmds_set_1);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"panel_cmds_set_1 failed ret = %d\n",
+			ret);
+	}
+
+	truly_write_buf_func(ret, truly_dcs_write_buf,
+		panel, LONG_PACKET,
+		ARRAY_SIZE(panel_cmds_set_2),
+		panel_cmds_set_2);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"panel_cmds_set_2 failed ret = %d\n",
+			ret);
+	}
+
+	truly_write_buf_func(ret, truly_dcs_write_buf,
+		panel, SHORT_PACKET,
+		ARRAY_SIZE(panel_cmds_set_3),
+		panel_cmds_set_3);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"panel_cmds_set_3 failed ret = %d\n",
+			ret);
+	}
+
+	/* As per DSI spec need to wait for 120ms
+	 * after sending the exit sleep DCS command
+	 */
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_EXIT_SLEEP_MODE);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"exit_sleep_mode cmd failed ret = %d\n",
+			ret);
+	}
+
+	msleep(120);
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_SET_DISPLAY_ON);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"set_display_on cmd failed ret = %d\n",
+			ret);
+	}
+
+	/* Adding 120ms delay before the start of
+	 * pixel stream to the panel after sending
+	 * the set_display_on DCS command
+	 */
+
+	msleep(120);
+
+	ctx->prepared = true;
+
+	return 0;
+}
+
+static int truly_wqxga_enable(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+
+	if (ctx->enabled)
+		return 0;
+
+	if (ctx->backlight) {
+		ctx->backlight->props.power = FB_BLANK_UNBLANK;
+		backlight_update_status(ctx->backlight);
+	}
+	ctx->enabled = true;
+
+	return 0;
+}
+
+static int truly_wqxga_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(connector->dev);
+	if (!mode) {
+		DRM_DEV_ERROR(ctx->dev,
+			"failed to create a new display mode\n");
+		return 0;
+	}
+
+	connector->display_info.width_mm = 74;
+	connector->display_info.height_mm = 131;
+	/* Copy the mode from the DT and add it */
+	drm_mode_copy(mode, &ctx->dm);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs truly_wqxga_drm_funcs = {
+	.disable = truly_wqxga_disable,
+	.unprepare = truly_wqxga_unprepare,
+	.prepare = truly_wqxga_prepare,
+	.enable = truly_wqxga_enable,
+	.get_modes = truly_wqxga_get_modes,
+};
+
+static int truly_wqxga_panel_add(struct truly_wqxga *ctx)
+{
+	struct device *dev = ctx->dev;
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
+		ctx->supplies[i].supply = regulator_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio)) {
+		DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",
+			PTR_ERR(ctx->reset_gpio));
+		return PTR_ERR(ctx->reset_gpio);
+	}
+
+	ctx->mode_gpio = devm_gpiod_get(dev, "mode", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->mode_gpio)) {
+		DRM_DEV_ERROR(dev, "cannot get mode gpio %ld\n",
+			PTR_ERR(ctx->mode_gpio));
+		return PTR_ERR(ctx->mode_gpio);
+	}
+
+	/* dual port */
+	gpiod_set_value(ctx->mode_gpio, 0);
+
+	ret = of_get_drm_display_mode(dev->of_node,
+		&ctx->dm, &ctx->bus_flags, 0);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "failed to get display mode\n");
+		return ret;
+	}
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = dev;
+	ctx->panel.funcs = &truly_wqxga_drm_funcs;
+	drm_panel_add(&ctx->panel);
+
+	return 0;
+}
+
+static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct truly_wqxga *ctx;
+	struct mipi_dsi_device *dsi1_device;
+	struct device_node *dsi1;
+	struct mipi_dsi_host *dsi1_host;
+	struct mipi_dsi_device *dsi_dev;
+	int ret = 0;
+	int i;
+
+	const struct mipi_dsi_device_info info = {.type = "trulynt35597",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+
+	if (!ctx)
+		return -ENOMEM;
+
+	/* This device represents itself as one with
+	 * two input ports which are fed by the output
+	 * ports of the two DSI controllers . The DSI0
+	 * is the master controller and has most of the
+	 * panel related info in its child node.
+	 */
+
+	dsi1 = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
+	if (!dsi1) {
+		DRM_DEV_ERROR(dev,
+			"failed to get remote node for dsi1_device\n");
+		ret = -ENODEV;
+		goto err_get_remote;
+	}
+
+	dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
+	if (!dsi1_host) {
+		DRM_DEV_ERROR(dev, "failed to find dsi host\n");
+		ret = -EPROBE_DEFER;
+		goto err_host;
+	}
+
+	of_node_put(dsi1);
+
+	/* register the second DSI device */
+	dsi1_device = mipi_dsi_device_register_full(dsi1_host, &info);
+	if (IS_ERR(dsi1_device)) {
+		DRM_DEV_ERROR(dev, "failed to create dsi device\n");
+		ret = PTR_ERR(dsi);
+		goto err_dsi_device;
+	}
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	ctx->dev = dev;
+	ctx->dsi[0] = dsi;
+	ctx->dsi[1] = dsi1_device;
+
+	ret = truly_wqxga_panel_add(ctx);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to add panel\n");
+		goto err_panel_add;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+		dsi_dev = ctx->dsi[i];
+		dsi_dev->lanes = 4;
+		dsi_dev->format = MIPI_DSI_FMT_RGB888;
+		dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_LPM |
+			MIPI_DSI_CLOCK_NON_CONTINUOUS;
+		ret = mipi_dsi_attach(dsi_dev);
+		if (ret < 0) {
+			DRM_DEV_ERROR(dev,
+				"dsi attach failed i = %d\n", i);
+				goto err_dsi_attach;
+		}
+	}
+
+	return 0;
+
+err_dsi_attach:
+	drm_panel_remove(&ctx->panel);
+err_panel_add:
+	mipi_dsi_device_unregister(dsi1_device);
+err_dsi_device:
+err_host:
+	of_node_put(dsi1);
+err_get_remote:
+	return ret;
+}
+
+static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
+{
+	struct truly_wqxga *ctx = mipi_dsi_get_drvdata(dsi);
+
+	if (ctx->dsi[0])
+		mipi_dsi_detach(ctx->dsi[0]);
+	if (ctx->dsi[1]) {
+		mipi_dsi_detach(ctx->dsi[1]);
+		mipi_dsi_device_unregister(ctx->dsi[1]);
+	}
+
+	drm_panel_remove(&ctx->panel);
+	return 0;
+}
+
+static const struct of_device_id truly_wqxga_of_match[] = {
+	{ .compatible = "truly,nt35597", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, truly_wqxga_of_match);
+
+static struct mipi_dsi_driver truly_wqxga_driver = {
+	.driver = {
+		.name = "panel_truly_nt35597",
+		.of_match_table = truly_wqxga_of_match,
+	},
+	.probe = truly_wqxga_probe,
+	.remove = truly_wqxga_remove,
+};
+module_mipi_dsi_driver(truly_wqxga_driver);
+
+MODULE_DESCRIPTION("Truly NT35597 DSI Panel Driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/2] dt-bindings: Add Truly NT35597 panel bindings

[Abhinav Kumar]

From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>

Add the device tree bindings for Truly NT35597 panel.
This panel supports both single DSI and dual DSI.

However, this patch series supports only dual DSI.

Changes in v5:
  - None

Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
---
 .../devicetree/bindings/display/truly,nt35597.txt  | 69 ++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/truly,nt35597.txt

diff --git a/Documentation/devicetree/bindings/display/truly,nt35597.txt b/Documentation/devicetree/bindings/display/truly,nt35597.txt
new file mode 100644
index 0000000..5d297b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/truly,nt35597.txt
@@ -0,0 +1,69 @@
+Truly model NT35597 1440x2560 DSI Panel
+
+Required properties:
+- compatible: should be "truly,nt35597"
+- vdda-supply: phandle of the regulator that provides the supply voltage
+  Power IC supply
+- vdispp-supply: phandle of the regulator that provides the supply voltage
+  for positive LCD bias
+- vdispn-supply: phandle of the regulator that provides the supply voltage
+  for negative LCD bias
+- reset-gpios: phandle of gpio for reset line
+  This should be 8mA, gpio can be configured using mux, pinctrl, pinctrl-names
+- mode-gpios: phandle of the gpio for choosing the mode of the display
+  for single DSI or Dual DSI
+  This should be low for dual DSI and high for single DSI mode
+- display-timings: Node for the Panel timings
+- ports: This device has two video ports driven by two DSIs. Their connections
+  are modelled using the OF graph bindings specified in
+  Documentation/devicetree/bindings/graph.txt.
+  - port@0: DSI input port driven by master DSI
+  - port@1: DSI input port driven by secondary DSI
+
+Example:
+
+	dsi@ae94000 {
+		panel@0 {
+			compatible = "truly,nt35597";
+			reg = <0>;
+			vdda-supply = <&pm8998_l14>;
+			vdispp-supply = <&lab_regulator>;
+			vdispn-supply = <&ibb_regulator>;
+			pinctrl-names = "default", "suspend";
+			pinctrl-0 = <&dpu_dsi_active>;
+			pinctrl-1 = <&dpu_dsi_suspend>;
+
+			reset-gpios = <&tlmm 6 0>;
+			mode-gpios = <&tlmm 52 0>;
+			display-timings {
+				timing0: timing-0 {
+					clock-frequency = <268316138>;
+					hactive = <1440>;
+					vactive = <2560>;
+					hfront-porch = <200>;
+					hback-porch = <64>;
+					hsync-len = <32>;
+					vfront-porch = <8>;
+					vback-porch = <7>;
+					vsync-len = <1>;
+				};
+			};
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					panel0_in: endpoint {
+						remote-endpoint = <&dsi0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					panel1_in: endpoint {
+						remote-endpoint = <&dsi1_out>;
+					};
+				};
+			};
+		};
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[Linus Walleij]

On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> wrote:

Hi Abhinav,

> From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
>
> Add support for Truly NT35597 panel used
> in MSM reference platforms.
>
> This panel supports both single DSI and dual DSI
> modes.
>
> However, this patch series adds support only for
> dual DSI mode.
>
> Changes in v5:
> - Added comments for the delays
> - Fix error messages and return code
> - Start using backlight_enable/disable helpers
> - Start using ARRAY_SIZE everywhere
> - Split the panel commands into three sets to
>   remove redundant structure fields and simplify
>   the DCS command sending method
> - Use of_get_drm_display_mode() and simplify
>   get_modes function
> - Remove truly_wqxga_panel_del and do necessary
>   cleanup
> - Replace dev_err with DRM_DEV_ERROR
>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>

Overall this driver looks good to me.

Just a question:

> +struct cmd_set panel_cmds_set_1[] = {
> +       /* CMD2_P0 */
> +       { { 0xff, 0x20 } },
> +       { { 0xfb, 0x01 } },
> +       { { 0x00, 0x01 } },

This is what we call a jam table, I guess "magic init sequence".

There are some comments on what the different sections do, but in
cases like this where there is no public datasheet, it would be nice
to use some #defines rather than opaque hex codes, if you know what
the different commands actually mean.

This is in order to help others with hacking on the driver.

If you don't have more info than this it's fine, just asking.

> +       /* Resolution:1440x2560 */
> +       { { 0x72, 0x02 } },

This is for example quite hard-coded. One gets the idea that the
resolution is dynamic and that this is not really a panel per se but
a panel driver, so the Truly NT35597 is not a panel but a panel driver
that can be configured to be used with several physical panels.

Compare to other panel drivers such as Ilitek ILI9322 that is in this
driver dir. There I make it a bit more transparent what the panel driver
is actually doing on the inside, so if we find it is used with other
physical panels we can reuse the code more easily.

> +       truly_write_buf_func(ret, truly_dcs_write_buf,
> +               panel, SHORT_PACKET,
> +               ARRAY_SIZE(panel_cmds_set_1),
> +               panel_cmds_set_1);

Instead of calling these "cmd_set_1" name them after what the
command set actually does so we can follow the init flow.
If you don't know what the commands do you could as well
call it "magic 1", "magic 2" etc so we know it is magic.

> +static const struct of_device_id truly_wqxga_of_match[] = {
> +       { .compatible = "truly,nt35597", },

If this is a panel driver that not only configurable for wqxga but actually
also other resolutions this is misleading.

I suspect this is indeed a panel driver and not a panel with integrated
driver. I think the best is to define two compatible strings like
we do for ILI9322:
"truly,nt35597", "qcom,reference-design-name-display";

The command sequence is probably just applicable for these Qcom
reference designs with a certain physical display, unless the display
controller can self-describe, e.g. by electric straps.

So only run these command sets for wqxga etc for this specific
qcom reference design, bail out if subsystem (reference design)
compatible string is not defined for now and match on the reference
design-display compatible.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 2/2] dt-bindings: Add Truly NT35597 panel bindings

[Linus Walleij]

On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> wrote:

> From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
>
> Add the device tree bindings for Truly NT35597 panel.
> This panel supports both single DSI and dual DSI.

I do not think this is a panel but a panel driver that can be used
with several physical panels. Can you confirm?

I suspect this since the command sequence in the driver seems
to contain a command for setting up the actual resolution and
a bunch of clocking properties for the physical panel.

> +Required properties:
> +- compatible: should be "truly,nt35597"

As with eg ili9322 I think this should have dual compatible strings
identifying the system it is used with since the resolution, clocking
etc is apparently
actually configurable.

compatible:
  "truly,nt35597", "qcom,reference-design1-name-display";
  "truly,nt35597", "qcom,reference-design2-name-display";

Then you send the command setting up resolution etc only for that
one system.

> +- vdda-supply: phandle of the regulator that provides the supply voltage
> +  Power IC supply
> +- vdispp-supply: phandle of the regulator that provides the supply voltage
> +  for positive LCD bias
> +- vdispn-supply: phandle of the regulator that provides the supply voltage
> +  for negative LCD bias
> +- reset-gpios: phandle of gpio for reset line
> +  This should be 8mA, gpio can be configured using mux, pinctrl, pinctrl-names
> +- mode-gpios: phandle of the gpio for choosing the mode of the display
> +  for single DSI or Dual DSI
> +  This should be low for dual DSI and high for single DSI mode
> +- display-timings: Node for the Panel timings

I don't think this belongs in the device tree at all.

Provide the timings from the driver based on the compatible string
instead, as you actually send commands to set up a certain timing in
the display controller.

(See ili9322 driver for an example of how I do this.)

> +- ports: This device has two video ports driven by two DSIs. Their connections
> +  are modelled using the OF graph bindings specified in
> +  Documentation/devicetree/bindings/graph.txt.
> +  - port@0: DSI input port driven by master DSI
> +  - port@1: DSI input port driven by secondary DSI

The rest seems fine.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[abhinavk]

Hi Linus

Thanks for your valuable comments.

We agree with your ideas and will address them.

Some details below on how we will address them.

Thanks

Abhinav
On 2018-08-03 04:03, Linus Walleij wrote:
> On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> 
> wrote:
> 
> Hi Abhinav,
> 
>> From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
>> 
>> Add support for Truly NT35597 panel used
>> in MSM reference platforms.
>> 
>> This panel supports both single DSI and dual DSI
>> modes.
>> 
>> However, this patch series adds support only for
>> dual DSI mode.
>> 
>> Changes in v5:
>> - Added comments for the delays
>> - Fix error messages and return code
>> - Start using backlight_enable/disable helpers
>> - Start using ARRAY_SIZE everywhere
>> - Split the panel commands into three sets to
>>   remove redundant structure fields and simplify
>>   the DCS command sending method
>> - Use of_get_drm_display_mode() and simplify
>>   get_modes function
>> - Remove truly_wqxga_panel_del and do necessary
>>   cleanup
>> - Replace dev_err with DRM_DEV_ERROR
>> 
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> 
> Overall this driver looks good to me.
> 
> Just a question:
> 
>> +struct cmd_set panel_cmds_set_1[] = {
>> +       /* CMD2_P0 */
>> +       { { 0xff, 0x20 } },
>> +       { { 0xfb, 0x01 } },
>> +       { { 0x00, 0x01 } },
> 
> This is what we call a jam table, I guess "magic init sequence".
> 
> There are some comments on what the different sections do, but in
> cases like this where there is no public datasheet, it would be nice
> to use some #defines rather than opaque hex codes, if you know what
> the different commands actually mean.
> 
> This is in order to help others with hacking on the driver.
> 
> If you don't have more info than this it's fine, just asking.
> 
Unfortunately, I do not have more info than this on each of them.
The documentation I have does not speak more in detail about what
each command does.

>> +       /* Resolution:1440x2560 */
>> +       { { 0x72, 0x02 } },
> 
> This is for example quite hard-coded. One gets the idea that the
> resolution is dynamic and that this is not really a panel per se but
> a panel driver, so the Truly NT35597 is not a panel but a panel driver
> that can be configured to be used with several physical panels.
> 
> Compare to other panel drivers such as Ilitek ILI9322 that is in this
> driver dir. There I make it a bit more transparent what the panel 
> driver
> is actually doing on the inside, so if we find it is used with other
> physical panels we can reuse the code more easily.
> 
>> +       truly_write_buf_func(ret, truly_dcs_write_buf,
>> +               panel, SHORT_PACKET,
>> +               ARRAY_SIZE(panel_cmds_set_1),
>> +               panel_cmds_set_1);
> 
> Instead of calling these "cmd_set_1" name them after what the
> command set actually does so we can follow the init flow.
> If you don't know what the commands do you could as well
> call it "magic 1", "magic 2" etc so we know it is magic.
> 
After going through the documentation I have, Yes i agree that
this is a panel driver and can support other panels/resolutions.

Yes, since I dont have more documentation to add here will change
these sets to magic_set_1, magic_set_2 etc.

>> +static const struct of_device_id truly_wqxga_of_match[] = {
>> +       { .compatible = "truly,nt35597", },
> 
> If this is a panel driver that not only configurable for wqxga but 
> actually
> also other resolutions this is misleading.
> 
> I suspect this is indeed a panel driver and not a panel with integrated
> driver. I think the best is to define two compatible strings like
> we do for ILI9322:
> "truly,nt35597", "qcom,reference-design-name-display";
> 
> The command sequence is probably just applicable for these Qcom
> reference designs with a certain physical display, unless the display
> controller can self-describe, e.g. by electric straps.
> 
> So only run these command sets for wqxga etc for this specific
> qcom reference design, bail out if subsystem (reference design)
> compatible string is not defined for now and match on the reference
> design-display compatible.
> 
Yes, we agree to have two compatible strings, and shall bail out for now
if it doesnt match our reference design one similar to what is present 
here:

https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-ilitek-ili9322.c#L757

> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 2/2] dt-bindings: Add Truly NT35597 panel bindings

[abhinavk]

Hi Linus

Thanks for your valuable comments.

Yes, we agree with your comments here and will address them.

Some details below on how we will take care of it.

Thanks

Abhinav

On 2018-08-03 04:20, Linus Walleij wrote:
> On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> 
> wrote:
> 
>> From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
>> 
>> Add the device tree bindings for Truly NT35597 panel.
>> This panel supports both single DSI and dual DSI.
> 
> I do not think this is a panel but a panel driver that can be used
> with several physical panels. Can you confirm?
Yes, from the documentation I have I can see that this is a panel
driver and can support other panels/resolutions.
> 
> I suspect this since the command sequence in the driver seems
> to contain a command for setting up the actual resolution and
> a bunch of clocking properties for the physical panel.
> 
>> +Required properties:
>> +- compatible: should be "truly,nt35597"
> 
> As with eg ili9322 I think this should have dual compatible strings
> identifying the system it is used with since the resolution, clocking
> etc is apparently
> actually configurable.
> 
> compatible:
>   "truly,nt35597", "qcom,reference-design1-name-display";
>   "truly,nt35597", "qcom,reference-design2-name-display";
> 
> Then you send the command setting up resolution etc only for that
> one system.
> 
Yes, I agree we will can have dual compatible strings and we will pick
resolution/modes based on the compatible string similar to this:

https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-ilitek-ili9322.c#L659

>> +- vdda-supply: phandle of the regulator that provides the supply 
>> voltage
>> +  Power IC supply
>> +- vdispp-supply: phandle of the regulator that provides the supply 
>> voltage
>> +  for positive LCD bias
>> +- vdispn-supply: phandle of the regulator that provides the supply 
>> voltage
>> +  for negative LCD bias
>> +- reset-gpios: phandle of gpio for reset line
>> +  This should be 8mA, gpio can be configured using mux, pinctrl, 
>> pinctrl-names
>> +- mode-gpios: phandle of the gpio for choosing the mode of the 
>> display
>> +  for single DSI or Dual DSI
>> +  This should be low for dual DSI and high for single DSI mode
>> +- display-timings: Node for the Panel timings
> 
> I don't think this belongs in the device tree at all.
> 
> Provide the timings from the driver based on the compatible string
> instead, as you actually send commands to set up a certain timing in
> the display controller.
> 
> (See ili9322 driver for an example of how I do this.)

Yes, will follow the example of ili9322 and do the same.

> 
>> +- ports: This device has two video ports driven by two DSIs. Their 
>> connections
>> +  are modelled using the OF graph bindings specified in
>> +  Documentation/devicetree/bindings/graph.txt.
>> +  - port@0: DSI input port driven by master DSI
>> +  - port@1: DSI input port driven by secondary DSI
> 
> The rest seems fine.
> 
> Yours,
> Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 4305 bytes --]

On Fri, Aug 03, 2018 at 01:03:45PM +0200, Linus Walleij wrote:
> On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> wrote:
> 
> Hi Abhinav,
> 
> > From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
> >
> > Add support for Truly NT35597 panel used
> > in MSM reference platforms.
> >
> > This panel supports both single DSI and dual DSI
> > modes.
> >
> > However, this patch series adds support only for
> > dual DSI mode.
> >
> > Changes in v5:
> > - Added comments for the delays
> > - Fix error messages and return code
> > - Start using backlight_enable/disable helpers
> > - Start using ARRAY_SIZE everywhere
> > - Split the panel commands into three sets to
> >   remove redundant structure fields and simplify
> >   the DCS command sending method
> > - Use of_get_drm_display_mode() and simplify
> >   get_modes function
> > - Remove truly_wqxga_panel_del and do necessary
> >   cleanup
> > - Replace dev_err with DRM_DEV_ERROR
> >
> > Signed-off-by: Archit Taneja <architt@codeaurora.org>
> > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> 
> Overall this driver looks good to me.
> 
> Just a question:
> 
> > +struct cmd_set panel_cmds_set_1[] = {
> > +       /* CMD2_P0 */
> > +       { { 0xff, 0x20 } },
> > +       { { 0xfb, 0x01 } },
> > +       { { 0x00, 0x01 } },
> 
> This is what we call a jam table, I guess "magic init sequence".
> 
> There are some comments on what the different sections do, but in
> cases like this where there is no public datasheet, it would be nice
> to use some #defines rather than opaque hex codes, if you know what
> the different commands actually mean.
> 
> This is in order to help others with hacking on the driver.
> 
> If you don't have more info than this it's fine, just asking.
> 
> > +       /* Resolution:1440x2560 */
> > +       { { 0x72, 0x02 } },
> 
> This is for example quite hard-coded. One gets the idea that the
> resolution is dynamic and that this is not really a panel per se but
> a panel driver, so the Truly NT35597 is not a panel but a panel driver
> that can be configured to be used with several physical panels.
> 
> Compare to other panel drivers such as Ilitek ILI9322 that is in this
> driver dir. There I make it a bit more transparent what the panel driver
> is actually doing on the inside, so if we find it is used with other
> physical panels we can reuse the code more easily.
> 
> > +       truly_write_buf_func(ret, truly_dcs_write_buf,
> > +               panel, SHORT_PACKET,
> > +               ARRAY_SIZE(panel_cmds_set_1),
> > +               panel_cmds_set_1);
> 
> Instead of calling these "cmd_set_1" name them after what the
> command set actually does so we can follow the init flow.
> If you don't know what the commands do you could as well
> call it "magic 1", "magic 2" etc so we know it is magic.
> 
> > +static const struct of_device_id truly_wqxga_of_match[] = {
> > +       { .compatible = "truly,nt35597", },
> 
> If this is a panel driver that not only configurable for wqxga but actually
> also other resolutions this is misleading.
> 
> I suspect this is indeed a panel driver and not a panel with integrated
> driver. I think the best is to define two compatible strings like
> we do for ILI9322:
> "truly,nt35597", "qcom,reference-design-name-display";

I don't understand why we need the two compatible strings for this.
Having "truly,nt35597" isn't quite correct in that case, because in
itself that doesn't contain enough information for any programming.

If that chip can indeed be used to drive different panels, what we
really need is the a compatible string that describes the complete
assembly. In the driver we could then rely on parameterized common
code that the panel driver can call into in order to program the
driver chip.

Note that a driver is assumed to know what to do with a device that
it gets bound to based on the compatible string. If some OS implements a
driver for "truly,nt35597" but not "qcom,reference-design-name-display",
then what is it supposed to do if it encounters the above list of
compatible strings? It can't really program a mode because its missing
essential information such as resolution or timings.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[Sean Paul]

On Thu, Aug 09, 2018 at 12:53:49PM +0200, Thierry Reding wrote:
> On Fri, Aug 03, 2018 at 01:03:45PM +0200, Linus Walleij wrote:
> > On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> wrote:
> > 
> > Hi Abhinav,
> > 
> > > From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
> > >
> > > Add support for Truly NT35597 panel used
> > > in MSM reference platforms.
> > >
> > > This panel supports both single DSI and dual DSI
> > > modes.
> > >
> > > However, this patch series adds support only for
> > > dual DSI mode.
> > >
> > > Changes in v5:
> > > - Added comments for the delays
> > > - Fix error messages and return code
> > > - Start using backlight_enable/disable helpers
> > > - Start using ARRAY_SIZE everywhere
> > > - Split the panel commands into three sets to
> > >   remove redundant structure fields and simplify
> > >   the DCS command sending method
> > > - Use of_get_drm_display_mode() and simplify
> > >   get_modes function
> > > - Remove truly_wqxga_panel_del and do necessary
> > >   cleanup
> > > - Replace dev_err with DRM_DEV_ERROR
> > >
> > > Signed-off-by: Archit Taneja <architt@codeaurora.org>
> > > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> > 
> > Overall this driver looks good to me.
> > 
> > Just a question:
> > 
> > > +struct cmd_set panel_cmds_set_1[] = {
> > > +       /* CMD2_P0 */
> > > +       { { 0xff, 0x20 } },
> > > +       { { 0xfb, 0x01 } },
> > > +       { { 0x00, 0x01 } },
> > 
> > This is what we call a jam table, I guess "magic init sequence".
> > 
> > There are some comments on what the different sections do, but in
> > cases like this where there is no public datasheet, it would be nice
> > to use some #defines rather than opaque hex codes, if you know what
> > the different commands actually mean.
> > 
> > This is in order to help others with hacking on the driver.
> > 
> > If you don't have more info than this it's fine, just asking.
> > 
> > > +       /* Resolution:1440x2560 */
> > > +       { { 0x72, 0x02 } },
> > 
> > This is for example quite hard-coded. One gets the idea that the
> > resolution is dynamic and that this is not really a panel per se but
> > a panel driver, so the Truly NT35597 is not a panel but a panel driver
> > that can be configured to be used with several physical panels.
> > 
> > Compare to other panel drivers such as Ilitek ILI9322 that is in this
> > driver dir. There I make it a bit more transparent what the panel driver
> > is actually doing on the inside, so if we find it is used with other
> > physical panels we can reuse the code more easily.
> > 
> > > +       truly_write_buf_func(ret, truly_dcs_write_buf,
> > > +               panel, SHORT_PACKET,
> > > +               ARRAY_SIZE(panel_cmds_set_1),
> > > +               panel_cmds_set_1);
> > 
> > Instead of calling these "cmd_set_1" name them after what the
> > command set actually does so we can follow the init flow.
> > If you don't know what the commands do you could as well
> > call it "magic 1", "magic 2" etc so we know it is magic.
> > 
> > > +static const struct of_device_id truly_wqxga_of_match[] = {
> > > +       { .compatible = "truly,nt35597", },
> > 
> > If this is a panel driver that not only configurable for wqxga but actually
> > also other resolutions this is misleading.
> > 
> > I suspect this is indeed a panel driver and not a panel with integrated
> > driver. I think the best is to define two compatible strings like
> > we do for ILI9322:
> > "truly,nt35597", "qcom,reference-design-name-display";
> 
> I don't understand why we need the two compatible strings for this.
> Having "truly,nt35597" isn't quite correct in that case, because in
> itself that doesn't contain enough information for any programming.
> 
> If that chip can indeed be used to drive different panels, what we
> really need is the a compatible string that describes the complete
> assembly. In the driver we could then rely on parameterized common
> code that the panel driver can call into in order to program the
> driver chip.

+1 (fwiw)

I doubt the generic string is ever going to be useful on its own, but it could
always be added later with a refactor of the code to tease out the generic bits.
Given that the TCON vendor won't give up the docs for the magic initialization
sequance, getting to a generic implementation is going to take a bit of work
anyways.

Sean

> 
> Note that a driver is assumed to know what to do with a device that
> it gets bound to based on the compatible string. If some OS implements a
> driver for "truly,nt35597" but not "qcom,reference-design-name-display",
> then what is it supposed to do if it encounters the above list of
> compatible strings? It can't really program a mode because its missing
> essential information such as resolution or timings.
> 
> Thierry



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[abhinavk]

Hi Thierry/Sean

Thanks for your comments.

Yes, I agree with you as well at the moment the generic "truly,nt35597" 
string doesnt have enough information to support multiple panels.

I am more comfortable renaming the compatible to 
"qcom,sdm845-mtp-2k-display" as we have all the pieces needed for this
compatible string.

When we have more information about what other panels this can support, 
we can generalize it with multiple strings.

Hi Linus

If you agree with us, I will go ahead with changing the compatible 
string of this driver from "truly,nt35597" to 
"qcom,sdm845-mtp-2k-display".

When we have more information of some other panel resolution which can 
be supported we can add other generic strings.

But yes, I will still implement the parts of moving the display timings 
from the DT to the panel driver which lay the groundwork
for extending this in the future to other resolutions.

Let me know your comments.

Thanks

Abhinav

On 2018-08-09 10:51, Sean Paul wrote:
> On Thu, Aug 09, 2018 at 12:53:49PM +0200, Thierry Reding wrote:
>> On Fri, Aug 03, 2018 at 01:03:45PM +0200, Linus Walleij wrote:
>> > On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> wrote:
>> >
>> > Hi Abhinav,
>> >
>> > > From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>
>> > >
>> > > Add support for Truly NT35597 panel used
>> > > in MSM reference platforms.
>> > >
>> > > This panel supports both single DSI and dual DSI
>> > > modes.
>> > >
>> > > However, this patch series adds support only for
>> > > dual DSI mode.
>> > >
>> > > Changes in v5:
>> > > - Added comments for the delays
>> > > - Fix error messages and return code
>> > > - Start using backlight_enable/disable helpers
>> > > - Start using ARRAY_SIZE everywhere
>> > > - Split the panel commands into three sets to
>> > >   remove redundant structure fields and simplify
>> > >   the DCS command sending method
>> > > - Use of_get_drm_display_mode() and simplify
>> > >   get_modes function
>> > > - Remove truly_wqxga_panel_del and do necessary
>> > >   cleanup
>> > > - Replace dev_err with DRM_DEV_ERROR
>> > >
>> > > Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> > > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
>> >
>> > Overall this driver looks good to me.
>> >
>> > Just a question:
>> >
>> > > +struct cmd_set panel_cmds_set_1[] = {
>> > > +       /* CMD2_P0 */
>> > > +       { { 0xff, 0x20 } },
>> > > +       { { 0xfb, 0x01 } },
>> > > +       { { 0x00, 0x01 } },
>> >
>> > This is what we call a jam table, I guess "magic init sequence".
>> >
>> > There are some comments on what the different sections do, but in
>> > cases like this where there is no public datasheet, it would be nice
>> > to use some #defines rather than opaque hex codes, if you know what
>> > the different commands actually mean.
>> >
>> > This is in order to help others with hacking on the driver.
>> >
>> > If you don't have more info than this it's fine, just asking.
>> >
>> > > +       /* Resolution:1440x2560 */
>> > > +       { { 0x72, 0x02 } },
>> >
>> > This is for example quite hard-coded. One gets the idea that the
>> > resolution is dynamic and that this is not really a panel per se but
>> > a panel driver, so the Truly NT35597 is not a panel but a panel driver
>> > that can be configured to be used with several physical panels.
>> >
>> > Compare to other panel drivers such as Ilitek ILI9322 that is in this
>> > driver dir. There I make it a bit more transparent what the panel driver
>> > is actually doing on the inside, so if we find it is used with other
>> > physical panels we can reuse the code more easily.
>> >
>> > > +       truly_write_buf_func(ret, truly_dcs_write_buf,
>> > > +               panel, SHORT_PACKET,
>> > > +               ARRAY_SIZE(panel_cmds_set_1),
>> > > +               panel_cmds_set_1);
>> >
>> > Instead of calling these "cmd_set_1" name them after what the
>> > command set actually does so we can follow the init flow.
>> > If you don't know what the commands do you could as well
>> > call it "magic 1", "magic 2" etc so we know it is magic.
>> >
>> > > +static const struct of_device_id truly_wqxga_of_match[] = {
>> > > +       { .compatible = "truly,nt35597", },
>> >
>> > If this is a panel driver that not only configurable for wqxga but actually
>> > also other resolutions this is misleading.
>> >
>> > I suspect this is indeed a panel driver and not a panel with integrated
>> > driver. I think the best is to define two compatible strings like
>> > we do for ILI9322:
>> > "truly,nt35597", "qcom,reference-design-name-display";
>> 
>> I don't understand why we need the two compatible strings for this.
>> Having "truly,nt35597" isn't quite correct in that case, because in
>> itself that doesn't contain enough information for any programming.
>> 
>> If that chip can indeed be used to drive different panels, what we
>> really need is the a compatible string that describes the complete
>> assembly. In the driver we could then rely on parameterized common
>> code that the panel driver can call into in order to program the
>> driver chip.
> 
> +1 (fwiw)
> 
> I doubt the generic string is ever going to be useful on its own, but 
> it could
> always be added later with a refactor of the code to tease out the 
> generic bits.
> Given that the TCON vendor won't give up the docs for the magic 
> initialization
> sequance, getting to a generic implementation is going to take a bit of 
> work
> anyways.
> 
> Sean
> 
>> 
>> Note that a driver is assumed to know what to do with a device that
>> it gets bound to based on the compatible string. If some OS implements 
>> a
>> driver for "truly,nt35597" but not 
>> "qcom,reference-design-name-display",
>> then what is it supposed to do if it encounters the above list of
>> compatible strings? It can't really program a mode because its missing
>> essential information such as resolution or timings.
>> 
>> Thierry
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[Linus Walleij]

On Thu, Aug 9, 2018 at 12:53 PM Thierry Reding <thierry.reding@gmail.com> wrote:

[Me]
> > I suspect this is indeed a panel driver and not a panel with integrated
> > driver. I think the best is to define two compatible strings like
> > we do for ILI9322:
> > "truly,nt35597", "qcom,reference-design-name-display";
>
> I don't understand why we need the two compatible strings for this.
> Having "truly,nt35597" isn't quite correct in that case, because in
> itself that doesn't contain enough information for any programming.
>
> If that chip can indeed be used to drive different panels, what we
> really need is the a compatible string that describes the complete
> assembly. In the driver we could then rely on parameterized common
> code that the panel driver can call into in order to program the
> driver chip.

Good point.

Rob asked the same for the TPO driver, but that case is
different.

For the TPO driver we had a situation where the display driver
was kind of self-identifying, using HW straps to set up the mode,
so the kernel driver only needed to read some registers to
get basic functionality up, so it could use the compatible
of the display driver itself.

In such cases it is useful with two compatible strings as there
are always quirks (that is why we always have two compatible
strings for SoCs) and a specific display/system may need e.g.
specific gamma correction values even if basic graphics come
up with just the display driver compatible strings.

So I would say it depens on how "plug-n-playsy" the display
driver is.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[kbuild test robot]

Hi abhinavk,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.18-rc7 next-20180802]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Abhinav-Kumar/drm-panel-Add-support-for-Truly-NT35597-panel/20180803-114716
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/panel/panel-truly-nt35597.c:68:16: sparse: symbol 'panel_cmds_set_1' was not declared. Should it be static?
>> drivers/gpu/drm/panel/panel-truly-nt35597.c:207:16: sparse: symbol 'panel_cmds_set_2' was not declared. Should it be static?
>> drivers/gpu/drm/panel/panel-truly-nt35597.c:212:16: sparse: symbol 'panel_cmds_set_3' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 1/2] drm/panel: Add support for Truly NT35597 panel

[Abhinav Kumar]

From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org>

Add support for Truly NT35597 panel used
in MSM reference platforms.

This panel supports both single DSI and dual DSI
modes.

However, this patch series adds support only for
dual DSI mode.

Changes in v5:
- Added comments for the delays
- Fix error messages and return code
- Start using backlight_enable/disable helpers
- Start using ARRAY_SIZE everywhere
- Split the panel commands into three sets to
  remove redundant structure fields and simplify
  the DCS command sending method
- Use of_get_drm_display_mode() and simplify
  get_modes function
- Remove truly_wqxga_panel_del and do necessary
  cleanup
- Replace dev_err with DRM_DEV_ERROR

Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
---
 drivers/gpu/drm/panel/Kconfig               |   8 +
 drivers/gpu/drm/panel/Makefile              |   1 +
 drivers/gpu/drm/panel/panel-truly-nt35597.c | 684 ++++++++++++++++++++++++++++
 3 files changed, 693 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35597.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 6020c30..7ae74c2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -186,4 +186,12 @@ config DRM_PANEL_SITRONIX_ST7789V
 	  Say Y here if you want to enable support for the Sitronix
 	  ST7789V controller for 240x320 LCD panels
 
+config DRM_PANEL_TRULY_NT35597_WQXGA
+	tristate "Truly WQXGA"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	select VIDEOMODE_HELPERS
+	help
+	  Say Y here if you want to enable support for Truly NT35597 WQXGA Dual DSI
+	  Video Mode panel
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 5ccaaa9..80fd19f 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
+obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c b/drivers/gpu/drm/panel/panel-truly-nt35597.c
new file mode 100644
index 0000000..e6c7662
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c
@@ -0,0 +1,684 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_mipi_dsi.h>
+
+static const char * const regulator_names[] = {
+	"vdda",
+	"vdispp",
+	"vdispn",
+};
+
+static unsigned long const regulator_enable_loads[] = {
+	62000,
+	100000,
+	100000,
+};
+
+static unsigned long const regulator_disable_loads[] = {
+	80,
+	100,
+	100,
+};
+
+struct truly_wqxga {
+	struct device *dev;
+	struct drm_panel panel;
+
+	struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *mode_gpio;
+
+	struct backlight_device *backlight;
+	struct drm_display_mode dm;
+	u32 bus_flags;
+	struct mipi_dsi_device *dsi[2];
+
+	bool prepared;
+	bool enabled;
+};
+
+static inline struct truly_wqxga *panel_to_truly_wqxga(struct drm_panel *panel)
+{
+	return container_of(panel, struct truly_wqxga, panel);
+}
+
+#define MAX_LEN	5
+#define SHORT_PACKET 2
+#define LONG_PACKET 4
+
+struct cmd_set {
+	u8 commands[MAX_LEN];
+};
+
+struct cmd_set panel_cmds_set_1[] = {
+	/* CMD2_P0 */
+	{ { 0xff, 0x20 } },
+	{ { 0xfb, 0x01 } },
+	{ { 0x00, 0x01 } },
+	{ { 0x01, 0x55 } },
+	{ { 0x02, 0x45 } },
+	{ { 0x05, 0x40 } },
+	{ { 0x06, 0x19 } },
+	{ { 0x07, 0x1e } },
+	{ { 0x0b, 0x73 } },
+	{ { 0x0c, 0x73 } },
+	{ { 0x0e, 0xb0 } },
+	{ { 0x0f, 0xae } },
+	{ { 0x11, 0xb8 } },
+	{ { 0x13, 0x00 } },
+	{ { 0x58, 0x80 } },
+	{ { 0x59, 0x01 } },
+	{ { 0x5a, 0x00 } },
+	{ { 0x5b, 0x01 } },
+	{ { 0x5c, 0x80 } },
+	{ { 0x5d, 0x81 } },
+	{ { 0x5e, 0x00 } },
+	{ { 0x5f, 0x01 } },
+	{ { 0x72, 0x11 } },
+	{ { 0x68, 0x03 } },
+	/* CMD2_P4 */
+	{ { 0xFF, 0x24 } },
+	{ { 0xFB, 0x01 } },
+	{ { 0x00, 0x1C } },
+	{ { 0x01, 0x0B } },
+	{ { 0x02, 0x0C } },
+	{ { 0x03, 0x01 } },
+	{ { 0x04, 0x0F } },
+	{ { 0x05, 0x10 } },
+	{ { 0x06, 0x10 } },
+	{ { 0x07, 0x10 } },
+	{ { 0x08, 0x89 } },
+	{ { 0x09, 0x8A } },
+	{ { 0x0A, 0x13 } },
+	{ { 0x0B, 0x13 } },
+	{ { 0x0C, 0x15 } },
+	{ { 0x0D, 0x15 } },
+	{ { 0x0E, 0x17 } },
+	{ { 0x0F, 0x17 } },
+	{ { 0x10, 0x1C } },
+	{ { 0x11, 0x0B } },
+	{ { 0x12, 0x0C } },
+	{ { 0x13, 0x01 } },
+	{ { 0x14, 0x0F } },
+	{ { 0x15, 0x10 } },
+	{ { 0x16, 0x10 } },
+	{ { 0x17, 0x10 } },
+	{ { 0x18, 0x89 } },
+	{ { 0x19, 0x8A } },
+	{ { 0x1A, 0x13 } },
+	{ { 0x1B, 0x13 } },
+	{ { 0x1C, 0x15 } },
+	{ { 0x1D, 0x15 } },
+	{ { 0x1E, 0x17 } },
+	{ { 0x1F, 0x17 } },
+	/* STV */
+	{ { 0x20, 0x40 } },
+	{ { 0x21, 0x01 } },
+	{ { 0x22, 0x00 } },
+	{ { 0x23, 0x40 } },
+	{ { 0x24, 0x40 } },
+	{ { 0x25, 0x6D } },
+	{ { 0x26, 0x40 } },
+	{ { 0x27, 0x40 } },
+	/* Vend */
+	{ { 0xE0, 0x00 } },
+	{ { 0xDC, 0x21 } },
+	{ { 0xDD, 0x22 } },
+	{ { 0xDE, 0x07 } },
+	{ { 0xDF, 0x07 } },
+	{ { 0xE3, 0x6D } },
+	{ { 0xE1, 0x07 } },
+	{ { 0xE2, 0x07 } },
+	/* UD */
+	{ { 0x29, 0xD8 } },
+	{ { 0x2A, 0x2A } },
+	/* CLK */
+	{ { 0x4B, 0x03 } },
+	{ { 0x4C, 0x11 } },
+	{ { 0x4D, 0x10 } },
+	{ { 0x4E, 0x01 } },
+	{ { 0x4F, 0x01 } },
+	{ { 0x50, 0x10 } },
+	{ { 0x51, 0x00 } },
+	{ { 0x52, 0x80 } },
+	{ { 0x53, 0x00 } },
+	{ { 0x56, 0x00 } },
+	{ { 0x54, 0x07 } },
+	{ { 0x58, 0x07 } },
+	{ { 0x55, 0x25 } },
+	/* Reset XDONB */
+	{ { 0x5B, 0x43 } },
+	{ { 0x5C, 0x00 } },
+	{ { 0x5F, 0x73 } },
+	{ { 0x60, 0x73 } },
+	{ { 0x63, 0x22 } },
+	{ { 0x64, 0x00 } },
+	{ { 0x67, 0x08 } },
+	{ { 0x68, 0x04 } },
+	/* Resolution:1440x2560 */
+	{ { 0x72, 0x02 } },
+	/* mux */
+	{ { 0x7A, 0x80 } },
+	{ { 0x7B, 0x91 } },
+	{ { 0x7C, 0xD8 } },
+	{ { 0x7D, 0x60 } },
+	{ { 0x7F, 0x15 } },
+	{ { 0x75, 0x15 } },
+	/* ABOFF */
+	{ { 0xB3, 0xC0 } },
+	{ { 0xB4, 0x00 } },
+	{ { 0xB5, 0x00 } },
+	/* Source EQ */
+	{ { 0x78, 0x00 } },
+	{ { 0x79, 0x00 } },
+	{ { 0x80, 0x00 } },
+	{ { 0x83, 0x00 } },
+	/* FP BP */
+	{ { 0x93, 0x0A } },
+	{ { 0x94, 0x0A } },
+	/* Inversion Type */
+	{ { 0x8A, 0x00 } },
+	{ { 0x9B, 0xFF } },
+	/* IMGSWAP =1 @PortSwap=1 */
+	{ { 0x9D, 0xB0 } },
+	{ { 0x9F, 0x63 } },
+	{ { 0x98, 0x10 } },
+	/* FRM */
+	{ { 0xEC, 0x00 } },
+	/* CMD1 */
+	{ { 0xFF, 0x10 } },
+};
+
+struct cmd_set panel_cmds_set_2[] = {
+	/* VBP+VSA=,VFP = 10H */
+	{ { 0x3B, 0x03, 0x0A, 0x0A } },
+};
+
+struct cmd_set panel_cmds_set_3[] = {
+	/* FTE on */
+	{ { 0x35, 0x00 } },
+	/* EN_BK =1(auto black) */
+	{ { 0xE5, 0x01 } },
+	/* CMD mode(10) VDO mode(03) */
+	{ { 0xBB, 0x03 } },
+	/* Non Reload MTP */
+	{ { 0xFB, 0x01 } },
+};
+
+static int truly_dcs_write(struct drm_panel *panel, u32 command)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+		ret = mipi_dsi_dcs_write(ctx->dsi[i],
+			command, NULL, 0);
+		if (ret < 0) {
+			DRM_DEV_ERROR(ctx->dev,
+				"set_display_off cmd failed for dsi = %d\n",
+			i);
+		}
+	}
+
+	return ret;
+}
+
+static int truly_dcs_write_buf(struct drm_panel *panel,
+	u32 size, u32 num_cmds, struct cmd_set *buf)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret = 0;
+	int i, j;
+
+	for (i = 0; i < num_cmds; i++) {
+		for (j = 0; j < ARRAY_SIZE(ctx->dsi); j++) {
+			ret = mipi_dsi_dcs_write_buffer(ctx->dsi[j],
+					buf[i].commands,
+					size);
+			if (ret < 0) {
+				DRM_DEV_ERROR(ctx->dev,
+					"failed to tx cmd [%d], err: %d\n",
+					i, ret);
+				return ret;
+			}
+		}
+	}
+
+	return ret;
+}
+
+#define truly_call_write_func(ret, func, panel, command) {\
+	ret = func(panel, command);     \
+}
+
+#define truly_write_buf_func(ret, func, panel, size, cmds, buf) {\
+	ret = func(panel, size, cmds, buf); \
+}
+
+static int truly_wqxga_power_on(struct truly_wqxga *ctx)
+{
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+					regulator_enable_loads[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	/* Reset sequence of truly panel requires
+	 * the reset pin to be pulled high for 10ms
+	 * followed by a low period of 10ms and then
+	 * pulled high again
+	 */
+
+	gpiod_set_value(ctx->reset_gpio, 1);
+	msleep(20);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	msleep(20);
+	gpiod_set_value(ctx->reset_gpio, 1);
+
+	return 0;
+}
+
+static int truly_wqxga_power_off(struct truly_wqxga *ctx)
+{
+	int ret, i;
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+				regulator_disable_loads[i]);
+		if (ret)
+			return ret;
+	}
+
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static int truly_wqxga_disable(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret;
+
+	if (!ctx->enabled)
+		return 0;
+
+	if (ctx->backlight) {
+		ret = backlight_disable(ctx->backlight);
+		if (ret < 0)
+			DRM_DEV_ERROR(ctx->dev, "backlight disable failed %d\n",
+				ret);
+	}
+
+	ctx->enabled = false;
+	return 0;
+}
+
+static int truly_wqxga_unprepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret = 0;
+
+	if (!ctx->prepared)
+		return 0;
+
+	ctx->dsi[0]->mode_flags = 0;
+	ctx->dsi[1]->mode_flags = 0;
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_SET_DISPLAY_OFF);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"set_display_off cmd failed ret = %d\n",
+			ret);
+	}
+
+	/* 120ms delay required here as per DCS spec */
+	msleep(120);
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_ENTER_SLEEP_MODE);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"enter_sleep cmd failed ret = %d\n",
+			ret);
+	}
+
+	ret = truly_wqxga_power_off(ctx);
+	if (ret < 0)
+		DRM_DEV_ERROR(ctx->dev,
+			"truly_wqxga_power_off failed ret = %d\n",
+			ret);
+
+	ctx->prepared = false;
+	return ret;
+}
+
+static int truly_wqxga_prepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret;
+
+	if (ctx->prepared)
+		return 0;
+
+	ret = truly_wqxga_power_on(ctx);
+	if (ret < 0)
+		return ret;
+
+	ctx->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
+	ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	truly_write_buf_func(ret, truly_dcs_write_buf,
+		panel, SHORT_PACKET,
+		ARRAY_SIZE(panel_cmds_set_1),
+		panel_cmds_set_1);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"panel_cmds_set_1 failed ret = %d\n",
+			ret);
+	}
+
+	truly_write_buf_func(ret, truly_dcs_write_buf,
+		panel, LONG_PACKET,
+		ARRAY_SIZE(panel_cmds_set_2),
+		panel_cmds_set_2);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"panel_cmds_set_2 failed ret = %d\n",
+			ret);
+	}
+
+	truly_write_buf_func(ret, truly_dcs_write_buf,
+		panel, SHORT_PACKET,
+		ARRAY_SIZE(panel_cmds_set_3),
+		panel_cmds_set_3);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"panel_cmds_set_3 failed ret = %d\n",
+			ret);
+	}
+
+	/* As per DSI spec need to wait for 120ms
+	 * after sending the exit sleep DCS command
+	 */
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_EXIT_SLEEP_MODE);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"exit_sleep_mode cmd failed ret = %d\n",
+			ret);
+	}
+
+	msleep(120);
+
+	truly_call_write_func(ret, truly_dcs_write, panel,
+		MIPI_DCS_SET_DISPLAY_ON);
+
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			"set_display_on cmd failed ret = %d\n",
+			ret);
+	}
+
+	/* Adding 120ms delay before the start of
+	 * pixel stream to the panel after sending
+	 * the set_display_on DCS command
+	 */
+
+	msleep(120);
+
+	ctx->prepared = true;
+
+	return 0;
+}
+
+static int truly_wqxga_enable(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	int ret;
+
+	if (ctx->enabled)
+		return 0;
+
+	if (ctx->backlight) {
+		ret = backlight_enable(ctx->backlight);
+		if (ret < 0)
+			DRM_DEV_ERROR(ctx->dev, "backlight enable failed %d\n",
+						  ret);
+	}
+
+	ctx->enabled = true;
+
+	return 0;
+}
+
+static int truly_wqxga_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(connector->dev);
+	if (!mode) {
+		DRM_DEV_ERROR(ctx->dev,
+			"failed to create a new display mode\n");
+		return 0;
+	}
+
+	connector->display_info.width_mm = 74;
+	connector->display_info.height_mm = 131;
+	/* Copy the mode from the DT and add it */
+	drm_mode_copy(mode, &ctx->dm);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs truly_wqxga_drm_funcs = {
+	.disable = truly_wqxga_disable,
+	.unprepare = truly_wqxga_unprepare,
+	.prepare = truly_wqxga_prepare,
+	.enable = truly_wqxga_enable,
+	.get_modes = truly_wqxga_get_modes,
+};
+
+static int truly_wqxga_panel_add(struct truly_wqxga *ctx)
+{
+	struct device *dev = ctx->dev;
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
+		ctx->supplies[i].supply = regulator_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio)) {
+		DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",
+			PTR_ERR(ctx->reset_gpio));
+		return PTR_ERR(ctx->reset_gpio);
+	}
+
+	ctx->mode_gpio = devm_gpiod_get(dev, "mode", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->mode_gpio)) {
+		DRM_DEV_ERROR(dev, "cannot get mode gpio %ld\n",
+			PTR_ERR(ctx->mode_gpio));
+		return PTR_ERR(ctx->mode_gpio);
+	}
+
+	/* dual port */
+	gpiod_set_value(ctx->mode_gpio, 0);
+
+	ret = of_get_drm_display_mode(dev->of_node,
+		&ctx->dm, &ctx->bus_flags, 0);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "failed to get display mode\n");
+		return ret;
+	}
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = dev;
+	ctx->panel.funcs = &truly_wqxga_drm_funcs;
+	drm_panel_add(&ctx->panel);
+
+	return 0;
+}
+
+static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct truly_wqxga *ctx;
+	struct mipi_dsi_device *dsi1_device;
+	struct device_node *dsi1;
+	struct mipi_dsi_host *dsi1_host;
+	struct mipi_dsi_device *dsi_dev;
+	int ret = 0;
+	int i;
+
+	const struct mipi_dsi_device_info info = {.type = "trulynt35597",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+
+	if (!ctx)
+		return -ENOMEM;
+
+	/* This device represents itself as one with
+	 * two input ports which are fed by the output
+	 * ports of the two DSI controllers . The DSI0
+	 * is the master controller and has most of the
+	 * panel related info in its child node.
+	 */
+
+	dsi1 = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
+	if (!dsi1) {
+		DRM_DEV_ERROR(dev,
+			"failed to get remote node for dsi1_device\n");
+		ret = -ENODEV;
+		goto err_get_remote;
+	}
+
+	dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
+	if (!dsi1_host) {
+		DRM_DEV_ERROR(dev, "failed to find dsi host\n");
+		ret = -EPROBE_DEFER;
+		goto err_host;
+	}
+
+	of_node_put(dsi1);
+
+	/* register the second DSI device */
+	dsi1_device = mipi_dsi_device_register_full(dsi1_host, &info);
+	if (IS_ERR(dsi1_device)) {
+		DRM_DEV_ERROR(dev, "failed to create dsi device\n");
+		ret = PTR_ERR(dsi);
+		goto err_dsi_device;
+	}
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	ctx->dev = dev;
+	ctx->dsi[0] = dsi;
+	ctx->dsi[1] = dsi1_device;
+
+	ret = truly_wqxga_panel_add(ctx);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to add panel\n");
+		goto err_panel_add;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+		dsi_dev = ctx->dsi[i];
+		dsi_dev->lanes = 4;
+		dsi_dev->format = MIPI_DSI_FMT_RGB888;
+		dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_LPM |
+			MIPI_DSI_CLOCK_NON_CONTINUOUS;
+		ret = mipi_dsi_attach(dsi_dev);
+		if (ret < 0) {
+			DRM_DEV_ERROR(dev,
+				"dsi attach failed i = %d\n", i);
+				goto err_dsi_attach;
+		}
+	}
+
+	return 0;
+
+err_dsi_attach:
+	drm_panel_remove(&ctx->panel);
+err_panel_add:
+	mipi_dsi_device_unregister(dsi1_device);
+err_dsi_device:
+err_host:
+	of_node_put(dsi1);
+err_get_remote:
+	return ret;
+}
+
+static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
+{
+	struct truly_wqxga *ctx = mipi_dsi_get_drvdata(dsi);
+
+	if (ctx->dsi[0])
+		mipi_dsi_detach(ctx->dsi[0]);
+	if (ctx->dsi[1]) {
+		mipi_dsi_detach(ctx->dsi[1]);
+		mipi_dsi_device_unregister(ctx->dsi[1]);
+	}
+
+	drm_panel_remove(&ctx->panel);
+	return 0;
+}
+
+static const struct of_device_id truly_wqxga_of_match[] = {
+	{ .compatible = "truly,nt35597", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, truly_wqxga_of_match);
+
+static struct mipi_dsi_driver truly_wqxga_driver = {
+	.driver = {
+		.name = "panel_truly_nt35597",
+		.of_match_table = truly_wqxga_of_match,
+	},
+	.probe = truly_wqxga_probe,
+	.remove = truly_wqxga_remove,
+};
+module_mipi_dsi_driver(truly_wqxga_driver);
+
+MODULE_DESCRIPTION("Truly NT35597 DSI Panel Driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel