drm/panel: Add Truly Dual DSI video mode panel

PATCH In review Abhinav Kumar 4 participants 7 messages Apr 7, 2018 – Apr 16, 2018 3 messages not in the archive

Subsystems (5)

Device tree bindings DRM drivers DRM misc Arm Qualcomm mailing list +1 more subsystems

lore

Share & tools

lore

Abhinav Kumar

author

2018-04-07 07:06 UTC #0 lore raw

PATCH 1/2 drm/panel: Add Truly Dual DSI video mode panel

From: Archit Taneja <[email protected]>

Add support for truly dual DSI video mode panel
panel used in MSM reference platforms.

Signed-off-by Archit Taneja <[email protected]>

Signed-off-by Abhinav Kumar <[email protected]>

---
 .../bindings/display/truly,dual_wqxga.txt          |  47 ++
 drivers/gpu/drm/panel/Kconfig                      |   7 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-truly-dual-dsi.c       | 530 +++++++++++++++++++++
 4 files changed, 585 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/truly,dual_wqxga.txt
 create mode 100644 drivers/gpu/drm/panel/panel-truly-dual-dsi.c

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +47 -0

new file mode 100644
index 0000000..a1b24c1
@@ -0,0 +1,47 @@
+Truly model NT35597 1440x2560 DSI Panel
+
+Required properties:
+- compatible: should be "truly,dual_wqxga"
+- vdda-supply: phandle of the regulator that provides the supply voltage
+  Power IC supply
+- lab-supply: phandle of the regulator that provides the supply voltage
+  for LCD bias
+- ibb-supply: phandle of the regulator that provides the supply voltage
+  for 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
+- display-timings: Node for the Panel timings
+- link2: phandle to the secondary node of the panel
+
+Example:
+
+	dsi@ae94000 {
+		panel@0 {
+			compatible = "truly,dual_wqxga";
+			reg = <0>;
+			link2 = <&link2>;
+			vdda-supply = <&pm8998_l14>;
+
+			pinctrl-names = "default", "suspend";
+			pinctrl-0 = <&sde_dsi_active>;
+			pinctrl-1 = <&sde_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>;
+				};
+			};
+		};
+	};

drivers/gpu/drm/panel/Kconfig +7 -0

index 988048e..a63c3f7 100644
@@ -168,4 +168,11 @@ 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_WQXGA
+	tristate "Truly WQXGA"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	help
+	  Say Y here if you want to enable support for Truly WQXGA Dual DSI
+	  Video Mode panel
 endmenu

drivers/gpu/drm/panel/Makefile +1 -0

index 3d2a88d..64891f6 100644
@@ -17,3 +17,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_WQXGA) += panel-truly-dual-dsi.o

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +530 -0

530 lines not on this page · raw message

Signature · 7 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

A message that is not in the archive (sent off-list or not yet received): 1523084813-858-1-git-send-email-abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

Abhinav Kumar

author

2018-04-07 07:06 UTC #2 lore raw

PATCH 2/2 drm/panel: add backlight control support for truly panel

Signed-off-by Abhinav Kumar <[email protected]>

---
 drivers/gpu/drm/panel/panel-truly-dual-dsi.c | 96 +++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +94 -2

index 47891ee..5d0ef90 100644
@@ -14,6 +14,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/leds.h>
 
 #include <video/mipi_display.h>
 #include <video/of_videomode.h>

@@ -23,6 +24,9 @@
 #include <drm/drm_panel.h>
 #include <drm/drm_mipi_dsi.h>
 
+#define BL_NODE_NAME_SIZE 32
+#define PRIM_DISPLAY_NODE 0
+
 struct truly_wqxga {
 	struct device *dev;
 	struct drm_panel panel;

@@ -33,6 +37,8 @@ struct truly_wqxga {
 	struct gpio_desc *mode_gpio;
 
 	struct backlight_device *backlight;
+	/* WLED params */
+	struct led_trigger *wled;
 	struct videomode vm;
 
 	struct mipi_dsi_device *dsi[2];

@@ -447,6 +453,83 @@ static void truly_wqxga_panel_del(struct truly_wqxga *ctx)
 		put_device(&ctx->dsi[1]->dev);
 }
 
+static int truly_backlight_device_update_status(struct backlight_device *bd)
+{
+	int brightness;
+	int max_brightness;
+	int rc = 0;
+
+	struct truly_wqxga *ctx = dev_get_drvdata(&bd->dev);
+
+	brightness = bd->props.brightness;
+	max_brightness = bd->props.max_brightness;
+
+	if ((bd->props.power != FB_BLANK_UNBLANK) ||
+		(bd->props.state & BL_CORE_FBBLANK) ||
+		  (bd->props.state & BL_CORE_SUSPENDED))
+		brightness = 0;
+
+	if (brightness > max_brightness)
+		brightness = max_brightness;
+
+	/* Need to check WLED driver capability upstream */
+	if (ctx && ctx->wled)
+		led_trigger_event(ctx->wled, brightness);
+
+	return rc;
+}
+
+static int truly_backlight_device_get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static const struct backlight_ops truly_backlight_device_ops = {
+	.update_status = truly_backlight_device_update_status,
+	.get_brightness = truly_backlight_device_get_brightness,
+};
+
+static int truly_backlight_setup(struct truly_wqxga *ctx)
+{
+	struct backlight_properties props;
+	char bl_node_name[BL_NODE_NAME_SIZE];
+
+	if (!ctx) {
+		dev_err(ctx->dev, "invalid context\n");
+		return -EINVAL;
+	}
+
+	if (!ctx->backlight) {
+		memset(&props, 0, sizeof(props));
+		props.type = BACKLIGHT_RAW;
+		props.power = FB_BLANK_UNBLANK;
+		props.max_brightness = 4096;
+
+		snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
+				 PRIM_DISPLAY_NODE);
+
+		ctx->backlight =  backlight_device_register(bl_node_name,
+				ctx->dev, ctx,
+				&truly_backlight_device_ops, &props);
+
+		if (IS_ERR_OR_NULL(ctx->backlight)) {
+			pr_err("Failed to register backlight\n");
+			ctx->backlight = NULL;
+			return -ENODEV;
+		}
+
+		/* Register with the LED driver interface */
+		led_trigger_register_simple("bkl-trigger", &ctx->wled);
+
+		if (!ctx->wled) {
+			pr_err("backlight led registration failed\n");
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
 static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
 {
 	struct device *dev = &dsi->dev;

@@ -466,10 +549,11 @@ static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
 		secondary = of_find_mipi_dsi_device_by_node(np);
 		of_node_put(np);
 
+		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+
 		if (!secondary)
 			return -EPROBE_DEFER;
 
-		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 		if (!ctx) {
 			put_device(&secondary->dev);
 			return -ENOMEM;

@@ -485,6 +569,12 @@ static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
 			put_device(&secondary->dev);
 			return ret;
 		}
+
+		ret = truly_backlight_setup(ctx);
+		if (ret) {
+			put_device(&secondary->dev);
+			return ret;
+		}
 	}
 
 	ret = mipi_dsi_attach(dsi);

@@ -504,8 +594,10 @@ static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
 	mipi_dsi_detach(dsi);
 
 	/* delete panel only for the DSI1 interface */
-	if (ctx)
+	if (ctx) {
 		truly_wqxga_panel_del(ctx);
+		kfree(ctx);
+	}
 
 	return 0;
 }

Signature · 7 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

A message that is not in the archive (sent off-list or not yet received): 1523084813-858-2-git-send-email-abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

Sean Paul

2018-04-13 20:46 UTC #4 lore raw

Re: [DPU PATCH 2/2] drm/panel: add backlight control support for truly panel

Quoted 13 lines · Abhinav Kumar

Register truly panel as a backlight led device and provide methods to control its backlight operation.
Signed-off-by Abhinav Kumar <[email protected]>
---
 drivers/gpu/drm/panel/panel-truly-dual-dsi.c | 96 +++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +0 -0
index 47891ee..5d0ef90 100644

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 17

Quoted 4 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +1 -0

@@ -14,6 +14,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/leds.h>

Includes should be alphabetical.

Quoted 23 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +5 -0

 #include <video/mipi_display.h>
 #include <video/of_videomode.h>

@@ -23,6 +24,9 @@
 #include <drm/drm_panel.h>
 #include <drm/drm_mipi_dsi.h>
 
+#define BL_NODE_NAME_SIZE 32
+#define PRIM_DISPLAY_NODE 0
+
 struct truly_wqxga {
 	struct device *dev;
 	struct drm_panel panel;

@@ -33,6 +37,8 @@ struct truly_wqxga {
 	struct gpio_desc *mode_gpio;
 
 	struct backlight_device *backlight;
+	/* WLED params */
+	struct led_trigger *wled;
 	struct videomode vm;
 
 	struct mipi_dsi_device *dsi[2];

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 461

Quoted 9 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +6 -0

@@ -447,6 +453,83 @@ static void truly_wqxga_panel_del(struct truly_wqxga *ctx)
 		put_device(&ctx->dsi[1]->dev);
 }
 
+static int truly_backlight_device_update_status(struct backlight_device *bd)
+{
+	int brightness;
+	int max_brightness;
+	int rc = 0;
+

extra line

Quoted 5 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +5 -0

+	struct truly_wqxga *ctx = dev_get_drvdata(&bd->dev);
+
+	brightness = bd->props.brightness;
+	max_brightness = bd->props.max_brightness;
+

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 476

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +10 -0

+	if ((bd->props.power != FB_BLANK_UNBLANK) ||
+		(bd->props.state & BL_CORE_FBBLANK) ||
+		  (bd->props.state & BL_CORE_SUSPENDED))
+		brightness = 0;
+
+	if (brightness > max_brightness)
+		brightness = max_brightness;
+
+	/* Need to check WLED driver capability upstream */
+	if (ctx && ctx->wled)

ctx can't be NULL, so no need to check for that. And if ctx->wled is null, it doesn't seem like this function will do anything. So how about just not registering the backlight if wled == NULL (if that's possible).

Quoted 14 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +14 -0

+		led_trigger_event(ctx->wled, brightness);
+
+	return rc;
+}
+
+static int truly_backlight_device_get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static const struct backlight_ops truly_backlight_device_ops = {
+	.update_status = truly_backlight_device_update_status,
+	.get_brightness = truly_backlight_device_get_brightness,
+};

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 500

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +10 -0

+
+static int truly_backlight_setup(struct truly_wqxga *ctx)
+{
+	struct backlight_properties props;
+	char bl_node_name[BL_NODE_NAME_SIZE];
+
+	if (!ctx) {
+		dev_err(ctx->dev, "invalid context\n");
+		return -EINVAL;
+	}

This can't happen.

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 509

Quoted 9 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +9 -0

+
+	if (!ctx->backlight) {
+		memset(&props, 0, sizeof(props));
+		props.type = BACKLIGHT_RAW;
+		props.power = FB_BLANK_UNBLANK;
+		props.max_brightness = 4096;
+
+		snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
+				 PRIM_DISPLAY_NODE);

Given that PRIM_DISPLAY_NODE is always 0, this seems like overkill for a pretty generic name "panel0-backlight". So let's just call it "truly_backlight" in the register call.

Quoted 27 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +23 -0

+
+		ctx->backlight =  backlight_device_register(bl_node_name,
+				ctx->dev, ctx,
+				&truly_backlight_device_ops, &props);
+
+		if (IS_ERR_OR_NULL(ctx->backlight)) {
+			pr_err("Failed to register backlight\n");
+			ctx->backlight = NULL;
+			return -ENODEV;
+		}
+
+		/* Register with the LED driver interface */
+		led_trigger_register_simple("bkl-trigger", &ctx->wled);
+
+		if (!ctx->wled) {
+			pr_err("backlight led registration failed\n");
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
 static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
 {
 	struct device *dev = &dsi->dev;

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static int truly_wqxga_probe(struct mipi_dsi_device *dsi) · line 553

Quoted 5 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +2 -0

@@ -466,10 +549,11 @@ static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
 		secondary = of_find_mipi_dsi_device_by_node(np);
 		of_node_put(np);
 
+		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+

Why move this?

Quoted 36 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +9 -2

 		if (!secondary)
 			return -EPROBE_DEFER;
 
-		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 		if (!ctx) {
 			put_device(&secondary->dev);
 			return -ENOMEM;

@@ -485,6 +569,12 @@ static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
 			put_device(&secondary->dev);
 			return ret;
 		}
+
+		ret = truly_backlight_setup(ctx);
+		if (ret) {
+			put_device(&secondary->dev);
+			return ret;
+		}
 	}
 
 	ret = mipi_dsi_attach(dsi);

@@ -504,8 +594,10 @@ static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
 	mipi_dsi_detach(dsi);
 
 	/* delete panel only for the DSI1 interface */
-	if (ctx)
+	if (ctx) {
 		truly_wqxga_panel_del(ctx);
+		kfree(ctx);
+	}
 
 	return 0;
 }

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Signature · 5 lines

Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

[email protected]

author

2018-04-13 20:59 UTC #5 lore raw

Re: [DPU PATCH 2/2] drm/panel: add backlight control support for truly panel

Hi Sean

Thanks for the comments.

Some replies inline.

Quoted 10 lines · Sean Paul

Quoted 9 lines · Abhinav Kumar
Register truly panel as a backlight led device and provide methods to control its backlight operation.
Signed-off-by Abhinav Kumar <[email protected]>
---
 drivers/gpu/drm/panel/panel-truly-dual-dsi.c | 96 
+++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

Quoted 11 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 17
Quoted 9 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +1 -0
index 47891ee..5d0ef90 100644
@@ -14,6 +14,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/leds.h>
Includes should be alphabetical.

[Abhinav] Ok, will take care of this in v2.

Quoted 28 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +5 -0

 #include <video/mipi_display.h>
 #include <video/of_videomode.h>

@@ -23,6 +24,9 @@
 #include <drm/drm_panel.h>
 #include <drm/drm_mipi_dsi.h>
 
+#define BL_NODE_NAME_SIZE 32
+#define PRIM_DISPLAY_NODE 0
+
 struct truly_wqxga {
 	struct device *dev;
 	struct drm_panel panel;

@@ -33,6 +37,8 @@ struct truly_wqxga {
 	struct gpio_desc *mode_gpio;
 
 	struct backlight_device *backlight;
+	/* WLED params */
+	struct led_trigger *wled;
 	struct videomode vm;
 
 	struct mipi_dsi_device *dsi[2];

@@ -447,6 +453,83 @@ static void truly_wqxga_panel_del(struct 
 truly_wqxga *ctx)
 		put_device(&ctx->dsi[1]->dev);
 }

Quoted 9 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 461
Quoted 7 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +6 -0
+static int truly_backlight_device_update_status(struct 
 backlight_device *bd)
+{
+	int brightness;
+	int max_brightness;
+	int rc = 0;
+
extra line

[Abhinav] Ok, will remove it in v2.

Quoted 11 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +11 -0

+	struct truly_wqxga *ctx = dev_get_drvdata(&bd->dev);
+
+	brightness = bd->props.brightness;
+	max_brightness = bd->props.max_brightness;
+
+	if ((bd->props.power != FB_BLANK_UNBLANK) ||
+		(bd->props.state & BL_CORE_FBBLANK) ||
+		  (bd->props.state & BL_CORE_SUSPENDED))
+		brightness = 0;
+
+	if (brightness > max_brightness)

Quoted 8 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 476
Quoted 4 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +4 -0
+		brightness = max_brightness;
+
+	/* Need to check WLED driver capability upstream */
+	if (ctx && ctx->wled)
ctx can't be NULL, so no need to check for that. And if ctx->wled is null, it doesn't seem like this function will do anything. So how about just not registering the backlight if wled == NULL (if that's possible).

[Abhinav] Yes, will remove the NULL check.
We are registering the backlight in the backlight_setup(), this was more of an additional check, to make sure ctx->wled is present before we trigger. Not sure if we should register again here.

Quoted 20 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +18 -0

+		led_trigger_event(ctx->wled, brightness);
+
+	return rc;
+}
+
+static int truly_backlight_device_get_brightness(struct 
 backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static const struct backlight_ops truly_backlight_device_ops = {
+	.update_status = truly_backlight_device_update_status,
+	.get_brightness = truly_backlight_device_get_brightness,
+};
+
+static int truly_backlight_setup(struct truly_wqxga *ctx)
+{
+	struct backlight_properties props;

Quoted 8 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 500
Quoted 6 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +6 -0
+	char bl_node_name[BL_NODE_NAME_SIZE];
+
+	if (!ctx) {
+		dev_err(ctx->dev, "invalid context\n");
+		return -EINVAL;
+	}
This can't happen.

[Abhinav] Will remove it.

Quoted 15 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 509
Quoted 10 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +9 -0
+
+	if (!ctx->backlight) {
+		memset(&props, 0, sizeof(props));
+		props.type = BACKLIGHT_RAW;
+		props.power = FB_BLANK_UNBLANK;
+		props.max_brightness = 4096;
+
+		snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
+				 PRIM_DISPLAY_NODE);
Given that PRIM_DISPLAY_NODE is always 0, this seems like overkill for a pretty generic name "panel0-backlight". So let's just call it "truly_backlight" in the register call.

[Abhinav] The reason for keeping it "panel0-backlight" is because userspace is using this node name to write the backlight. Changing the name will need more changes in our userspace.

Quoted 31 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +23 -0

+
+		ctx->backlight =  backlight_device_register(bl_node_name,
+				ctx->dev, ctx,
+				&truly_backlight_device_ops, &props);
+
+		if (IS_ERR_OR_NULL(ctx->backlight)) {
+			pr_err("Failed to register backlight\n");
+			ctx->backlight = NULL;
+			return -ENODEV;
+		}
+
+		/* Register with the LED driver interface */
+		led_trigger_register_simple("bkl-trigger", &ctx->wled);
+
+		if (!ctx->wled) {
+			pr_err("backlight led registration failed\n");
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+

static int truly_wqxga_probe(struct mipi_dsi_device *dsi) {

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +0 -0

 	struct device *dev = &dsi->dev;

@@ -466,10 +549,11 @@ static int truly_wqxga_probe(struct 
 mipi_dsi_device *dsi)
 		secondary = of_find_mipi_dsi_device_by_node(np);
 		of_node_put(np);

Quoted 4 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c static int truly_wqxga_probe(struct mipi_dsi_device *dsi) · line 553
Quoted 2 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +2 -0
+		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+
Why move this?

[Abhinav] Thanks for catching this, yes not required. Will move it back.

Quoted 33 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +0 -1

 		if (!secondary)
 			return -EPROBE_DEFER;
 
-		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 		if (!ctx) {
 			put_device(&secondary->dev);

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +6 -0

 			return -ENOMEM;

@@ -485,6 +569,12 @@ static int truly_wqxga_probe(struct 
 mipi_dsi_device *dsi)
 			put_device(&secondary->dev);
 			return ret;
 		}
+
+		ret = truly_backlight_setup(ctx);
+		if (ret) {
+			put_device(&secondary->dev);
+			return ret;
+		}
 	}

drivers/gpu/drm/panel/panel-truly-dual-dsi.c +3 -1

 	ret = mipi_dsi_attach(dsi);

@@ -504,8 +594,10 @@ static int truly_wqxga_remove(struct 
 mipi_dsi_device *dsi)
 	mipi_dsi_detach(dsi);
 
 	/* delete panel only for the DSI1 interface */
-	if (ctx)
+	if (ctx) {
 		truly_wqxga_panel_del(ctx);
+		kfree(ctx);
+	}

Quoted 6 lines

Quoted 6 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +0 -0
 	return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

A message that is not in the archive (sent off-list or not yet received): 7d4804028f57b63c9d50f12743902b6b-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

Sean Paul

2018-04-16 17:00 UTC #7 lore raw

Re: [DPU PATCH 2/2] drm/panel: add backlight control support for truly panel

Quoted 13 lines · On Fri, Apr 13, 2018 at 01:59:29PM -0700, [email protected] wrote:

Hi Sean
Thanks for the comments.
Some replies inline.
Quoted 6 lines · Sean Paul
Quoted 5 lines · Abhinav Kumar
Register truly panel as a backlight led device and provide methods to control its backlight operation.
Signed-off-by Abhinav Kumar <[email protected]>
---

Quoted 15 lines

Quoted 6 lines
Quoted 6 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +6 -0
+	if (!ctx->backlight) {
+		memset(&props, 0, sizeof(props));
+		props.type = BACKLIGHT_RAW;
+		props.power = FB_BLANK_UNBLANK;
+		props.max_brightness = 4096;
+
Quoted 9 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c static void truly_wqxga_panel_del(struct truly_wqxga *ctx) · line 509
Quoted 2 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +2 -0
+		snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
+				 PRIM_DISPLAY_NODE);
Given that PRIM_DISPLAY_NODE is always 0, this seems like overkill for a pretty
generic name "panel0-backlight". So let's just call it "truly_backlight" in the
register call.

Quoted 6 lines

[Abhinav] The reason for keeping it "panel0-backlight" is because userspace is using
this node name to write the backlight. Changing the name will need more changes in our
userspace.

Unless the userspace is opensource (I'm guessing it's not?), that's not something we need to code around. It sounds like this is mostly a moot point given Bjorn's comments on the v2.

Sean

Signature · 5 lines

Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

Archit Taneja

2018-04-08 16:36 UTC #8 lore raw

In reply to a message not in the archive

Hi Abhinav,

Thanks for posting this driver. Some comments below.

Quoted 21 lines · Abhinav Kumar

From: Archit Taneja <[email protected]>
Add support for truly dual DSI video mode panel
panel used in MSM reference platforms >
Signed-off-by Archit Taneja <[email protected]>
Signed-off-by Abhinav Kumar <[email protected]>
---
  .../bindings/display/truly,dual_wqxga.txt          |  47 ++
  drivers/gpu/drm/panel/Kconfig                      |   7 +
  drivers/gpu/drm/panel/Makefile                     |   1 +
  drivers/gpu/drm/panel/panel-truly-dual-dsi.c       | 530 +++++++++++++++++++++
  4 files changed, 585 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/truly,dual_wqxga.txt create mode 100644 drivers/gpu/drm/panel/panel-truly-dual-dsi.c
Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +0 -0
new file mode 100644
index 0000000..a1b24c1

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt line 4

Quoted 4 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +4 -0
@@ -0,0 +1,47 @@
+Truly model NT35597 1440x2560 DSI Panel
+
+Required properties:
+- compatible: should be "truly,dual_wqxga"

The compatible string, kernel config and the driver file should be based on the panel model no. There can be many truly based panels that support wqxga. Something like "truly,nt35597" would be better.

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt line 10

Quoted 6 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +6 -0

+- vdda-supply: phandle of the regulator that provides the supply voltage
+  Power IC supply
+- lab-supply: phandle of the regulator that provides the supply voltage
+  for LCD bias
+- ibb-supply: phandle of the regulator that provides the supply voltage
+  for LCD bias

Both seem to have the same description. Aren't lab and ibb qualcomm specific terms? Could we use the pin names specified in the panel's data sheet?

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt line 14

Quoted 4 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +4 -0

+- 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

Could we describe here how to use this gpio? I.e, whether we need to set it to low for dual DSI, etc?

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt line 16

Quoted 2 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +2 -0
+- display-timings: Node for the Panel timings
+- link2: phandle to the secondary node of the panel

The link2 binding was a temporary hack we used. We should use the of-graph bindings to represent the two DSI input ports of the panel.

Quoted 62 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +31 -0

+
+Example:
+
+	dsi@ae94000 {
+		panel@0 {
+			compatible = "truly,dual_wqxga";
+			reg = <0>;
+			link2 = <&link2>;
+			vdda-supply = <&pm8998_l14>;
+
+			pinctrl-names = "default", "suspend";
+			pinctrl-0 = <&sde_dsi_active>;
+			pinctrl-1 = <&sde_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>;
+				};
+			};
+		};
+	};

drivers/gpu/drm/panel/Kconfig +7 -0

index 988048e..a63c3f7 100644
@@ -168,4 +168,11 @@ 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_WQXGA
+	tristate "Truly WQXGA"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	help
+	  Say Y here if you want to enable support for Truly WQXGA Dual DSI
+	  Video Mode panel
  endmenu

drivers/gpu/drm/panel/Makefile +1 -0

index 3d2a88d..64891f6 100644
@@ -17,3 +17,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_WQXGA) += panel-truly-dual-dsi.o

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +0 -0

new file mode 100644
index 0000000..47891ee

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 1

Quoted 1 line

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +1 -0
@@ -0,0 +1,530 @@
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved.

We should use the SPDX license headers here.

Quoted 52 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +52 -0

+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/gpio/consumer.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>
+
+struct truly_wqxga {
+	struct device *dev;
+	struct drm_panel panel;
+
+	struct regulator_bulk_data supplies[4];
+
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *mode_gpio;
+
+	struct backlight_device *backlight;
+	struct videomode vm;
+
+	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);
+}
+
+static int truly_wqxga_power_on(struct truly_wqxga *ctx)
+{
+	int ret;
+
+	ret = regulator_set_load(ctx->supplies[1].consumer, 62000);

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 63

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +10 -0

+	if (ret)
+		return ret;
+
+	ret = regulator_set_load(ctx->supplies[2].consumer, 100000);
+	if (ret)
+		return ret;
+
+	ret = regulator_set_load(ctx->supplies[3].consumer, 100000);
+	if (ret)
+		return ret;

We could make a const static struct array specifying the regulator names (to be used during probe) and their corresponding power on and power off loads. That should make things a bit cleaner.

Quoted 218 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +218 -0

+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	msleep(20);
+	gpiod_set_value(ctx->reset_gpio, 1);
+	usleep_range(20000, 20100);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(20000, 20100);
+	gpiod_set_value(ctx->reset_gpio, 1);
+	usleep_range(50000, 50100);
+
+	/* dual port */
+	gpiod_set_value(ctx->mode_gpio, 0);
+
+	return 0;
+}
+
+static int truly_wqxga_power_off(struct truly_wqxga *ctx)
+{
+	int ret;
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+	ret = regulator_set_load(ctx->supplies[1].consumer, 62000);
+	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);
+
+	if (!ctx->enabled)
+		return 0;
+
+	if (ctx->backlight) {
+		ctx->backlight->props.power = FB_BLANK_POWERDOWN;
+		backlight_update_status(ctx->backlight);
+	}
+
+	ctx->enabled = false;
+	return 0;
+}
+
+static int truly_wqxga_unprepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	struct mipi_dsi_device **dsis = ctx->dsi;
+	int ret = 0, i;
+
+	if (!ctx->prepared)
+		return 0;
+
+	dsis[0]->mode_flags = 0;
+	dsis[1]->mode_flags = 0;
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_set_display_off(dsis[i]) < 0)
+			ret = -ECOMM;
+	msleep(78);
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_enter_sleep_mode(dsis[i]) < 0)
+			ret = -ECOMM;
+	msleep(78);
+
+	truly_wqxga_power_off(ctx);
+
+	ctx->prepared = false;
+	return ret;
+}
+
+#define MAX_LEN	5
+struct {
+	u8 commands[MAX_LEN];
+	int size;
+} panel_cmds[] = { /* CMD2_P0 */
+		   { { 0xff, 0x20 }, 2 },
+		   { { 0xfb, 0x01 }, 2 },
+		   { { 0x00, 0x01 }, 2 },
+		   { { 0x01, 0x55 }, 2 },
+		   { { 0x02, 0x45 }, 2 },
+		   { { 0x05, 0x40 }, 2 },
+		   { { 0x06, 0x19 }, 2 },
+		   { { 0x07, 0x1e }, 2 },
+		   { { 0x0b, 0x73 }, 2 },
+		   { { 0x0c, 0x73 }, 2 },
+		   { { 0x0e, 0xb0 }, 2 },
+		   { { 0x0f, 0xae }, 2 },
+		   { { 0x11, 0xb8 }, 2 },
+		   { { 0x13, 0x00 }, 2 },
+		   { { 0x58, 0x80 }, 2 },
+		   { { 0x59, 0x01 }, 2 },
+		   { { 0x5a, 0x00 }, 2 },
+		   { { 0x5b, 0x01 }, 2 },
+		   { { 0x5c, 0x80 }, 2 },
+		   { { 0x5d, 0x81 }, 2 },
+		   { { 0x5e, 0x00 }, 2 },
+		   { { 0x5f, 0x01 }, 2 },
+		   { { 0x72, 0x11 }, 2 },
+		   { { 0x68, 0x03 }, 2 },
+		   /* CMD2_P4 */
+		   { { 0xFF, 0x24 }, 2 },
+		   { { 0xFB, 0x01 }, 2 },
+		   { { 0x00, 0x1C }, 2 },
+		   { { 0x01, 0x0B }, 2 },
+		   { { 0x02, 0x0C }, 2 },
+		   { { 0x03, 0x01 }, 2 },
+		   { { 0x04, 0x0F }, 2 },
+		   { { 0x05, 0x10 }, 2 },
+		   { { 0x06, 0x10 }, 2 },
+		   { { 0x07, 0x10 }, 2 },
+		   { { 0x08, 0x89 }, 2 },
+		   { { 0x09, 0x8A }, 2 },
+		   { { 0x0A, 0x13 }, 2 },
+		   { { 0x0B, 0x13 }, 2 },
+		   { { 0x0C, 0x15 }, 2 },
+		   { { 0x0D, 0x15 }, 2 },
+		   { { 0x0E, 0x17 }, 2 },
+		   { { 0x0F, 0x17 }, 2 },
+		   { { 0x10, 0x1C }, 2 },
+		   { { 0x11, 0x0B }, 2 },
+		   { { 0x12, 0x0C }, 2 },
+		   { { 0x13, 0x01 }, 2 },
+		   { { 0x14, 0x0F }, 2 },
+		   { { 0x15, 0x10 }, 2 },
+		   { { 0x16, 0x10 }, 2 },
+		   { { 0x17, 0x10 }, 2 },
+		   { { 0x18, 0x89 }, 2 },
+		   { { 0x19, 0x8A }, 2 },
+		   { { 0x1A, 0x13 }, 2 },
+		   { { 0x1B, 0x13 }, 2 },
+		   { { 0x1C, 0x15 }, 2 },
+		   { { 0x1D, 0x15 }, 2 },
+		   { { 0x1E, 0x17 }, 2 },
+		   { { 0x1F, 0x17 }, 2 },
+		   /* STV */
+		   { { 0x20, 0x40 }, 2 },
+		   { { 0x21, 0x01 }, 2 },
+		   { { 0x22, 0x00 }, 2 },
+		   { { 0x23, 0x40 }, 2 },
+		   { { 0x24, 0x40 }, 2 },
+		   { { 0x25, 0x6D }, 2 },
+		   { { 0x26, 0x40 }, 2 },
+		   { { 0x27, 0x40 }, 2 },
+		   /* Vend */
+		   { { 0xE0, 0x00 }, 2 },
+		   { { 0xDC, 0x21 }, 2 },
+		   { { 0xDD, 0x22 }, 2 },
+		   { { 0xDE, 0x07 }, 2 },
+		   { { 0xDF, 0x07 }, 2 },
+		   { { 0xE3, 0x6D }, 2 },
+		   { { 0xE1, 0x07 }, 2 },
+		   { { 0xE2, 0x07 }, 2 },
+		   /* UD */
+		   { { 0x29, 0xD8 }, 2 },
+		   { { 0x2A, 0x2A }, 2 },
+		   /* CLK */
+		   { { 0x4B, 0x03 }, 2 },
+		   { { 0x4C, 0x11 }, 2 },
+		   { { 0x4D, 0x10 }, 2 },
+		   { { 0x4E, 0x01 }, 2 },
+		   { { 0x4F, 0x01 }, 2 },
+		   { { 0x50, 0x10 }, 2 },
+		   { { 0x51, 0x00 }, 2 },
+		   { { 0x52, 0x80 }, 2 },
+		   { { 0x53, 0x00 }, 2 },
+		   { { 0x56, 0x00 }, 2 },
+		   { { 0x54, 0x07 }, 2 },
+		   { { 0x58, 0x07 }, 2 },
+		   { { 0x55, 0x25 }, 2 },
+		   /* Reset XDONB */
+		   { { 0x5B, 0x43 }, 2 },
+		   { { 0x5C, 0x00 }, 2 },
+		   { { 0x5F, 0x73 }, 2 },
+		   { { 0x60, 0x73 }, 2 },
+		   { { 0x63, 0x22 }, 2 },
+		   { { 0x64, 0x00 }, 2 },
+		   { { 0x67, 0x08 }, 2 },
+		   { { 0x68, 0x04 }, 2 },
+		   /* Resolution:1440x2560 */
+		   { { 0x72, 0x02 }, 2 },
+		   /* mux */
+		   { { 0x7A, 0x80 }, 2 },
+		   { { 0x7B, 0x91 }, 2 },
+		   { { 0x7C, 0xD8 }, 2 },
+		   { { 0x7D, 0x60 }, 2 },
+		   { { 0x7F, 0x15 }, 2 },
+		   { { 0x75, 0x15 }, 2 },
+		   /* ABOFF */
+		   { { 0xB3, 0xC0 }, 2 },
+		   { { 0xB4, 0x00 }, 2 },
+		   { { 0xB5, 0x00 }, 2 },
+		   /* Source EQ */
+		   { { 0x78, 0x00 }, 2 },
+		   { { 0x79, 0x00 }, 2 },
+		   { { 0x80, 0x00 }, 2 },
+		   { { 0x83, 0x00 }, 2 },
+		   /* FP BP */
+		   { { 0x93, 0x0A }, 2 },
+		   { { 0x94, 0x0A }, 2 },
+		   /* Inversion Type */
+		   { { 0x8A, 0x00 }, 2 },
+		   { { 0x9B, 0xFF }, 2 },
+		   /* IMGSWAP =1 @PortSwap=1 */
+		   { { 0x9D, 0xB0 }, 2 },
+		   { { 0x9F, 0x63 }, 2 },
+		   { { 0x98, 0x10 }, 2 },
+		   /* FRM */
+		   { { 0xEC, 0x00 }, 2 },
+		   /* CMD1 */
+		   { { 0xFF, 0x10 }, 2 },
+		    /* VBP+VSA=,VFP = 10H */
+		   { { 0x3B, 0x03, 0x0A, 0x0A, }, 4 },
+		   /* FTE on */

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 291

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +10 -0

+		   { { 0x35, 0x00 }, 2 },
+		   /* EN_BK =1(auto black) */
+		   { { 0xE5, 0x01 }, 2 },
+		   /* CMD mode(10) VDO mode(03) */
+		   { { 0xBB, 0x03 }, 2 },
+		   /* Non Reload MTP */
+		   { { 0xFB, 0x01 }, 2 },
+		   /* SlpOut + DispOn */
+		   //05 01 00 00 78 00 02 11 00
+		   //05 01 00 00 78 00 02 29 00

We can drop the commented lines. If possible, it would be nice to have some more description about the registers, but I can imagine that info being hard to retrieve.

Quoted 19 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +19 -0

+};
+
+static int truly_wqxga_prepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	struct mipi_dsi_device **dsis = ctx->dsi;
+	struct mipi_dsi_device *d;
+	int ret, i, j;
+
+	if (ctx->prepared)
+		return 0;
+
+	ret = truly_wqxga_power_on(ctx);
+	if (ret < 0)
+		return ret;
+
+	dsis[0]->mode_flags |= MIPI_DSI_MODE_LPM;
+	dsis[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 320

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +10 -0

+	for (j = 0; j < ARRAY_SIZE(panel_cmds); j++) {
+		for (i = 0; i < 2; i++) {
+			d = dsis[i];
+			ret = mipi_dsi_dcs_write_buffer(dsis[i],
+					panel_cmds[j].commands,
+					panel_cmds[j].size);
+			if (ret < 0) {
+				/* continue anyway */
+				dev_err(ctx->dev, "failed to cmd no %d, err: %d\n",
+						j, ret);

This may have been done for debug purposes. It would be probably better to bail out.

Quoted 158 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +158 -0

+			}
+		}
+	}
+
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_exit_sleep_mode(dsis[i]) < 0) {
+			dev_err(ctx->dev, "failed to exit sleep mode\n");
+			return -ECOMM;
+		}
+	msleep(78);
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_set_display_on(dsis[i]) < 0) {
+			dev_err(ctx->dev, "failed to send display on\n");
+			return -ECOMM;
+		}
+	msleep(78);
+
+	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) {
+		dev_err(ctx->dev, "failed to create a new display mode\n");
+		return 0;
+	}
+
+	drm_display_mode_from_videomode(&ctx->vm, mode);
+	connector->display_info.width_mm = 74;
+	connector->display_info.height_mm = 131;
+	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;
+
+	ctx->supplies[0].supply = "vddio";
+	ctx->supplies[1].supply = "vdda";
+	ctx->supplies[2].supply = "lab";
+	ctx->supplies[3].supply = "ibb";
+	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)) {
+		dev_err(dev, "cannot get reset-gpios %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)) {
+		dev_err(dev, "cannot get mode gpio %ld\n",
+			PTR_ERR(ctx->mode_gpio));
+		ctx->mode_gpio = NULL;
+		return PTR_ERR(ctx->mode_gpio);
+	}
+
+	ret = pinctrl_pm_select_default_state(dev);
+	if (ret) {
+		dev_err(dev, "%s: failed to set pinctrl default state, %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = of_get_videomode(dev->of_node, &ctx->vm, 0);
+	if (ret < 0)
+		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 void truly_wqxga_panel_del(struct truly_wqxga *ctx)
+{
+	if (ctx->panel.dev)
+		drm_panel_remove(&ctx->panel);
+
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	if (ctx->dsi[1])
+		put_device(&ctx->dsi[1]->dev);
+}
+
+static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct truly_wqxga *ctx;
+	struct mipi_dsi_device *secondary = NULL;
+	struct device_node *np;
+	int ret;
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_CLOCK_NON_CONTINUOUS |
+			  MIPI_DSI_MODE_LPM;
+
+	/* Only DSI-LINK1 node has "link2" entry. */
+	np = of_parse_phandle(dsi->dev.of_node, "link2", 0);
+	if (np) {
+		secondary = of_find_mipi_dsi_device_by_node(np);
+		of_node_put(np);
+
+		if (!secondary)
+			return -EPROBE_DEFER;
+
+		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+		if (!ctx) {
+			put_device(&secondary->dev);
+			return -ENOMEM;
+		}
+		mipi_dsi_set_drvdata(dsi, ctx);
+

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 488

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +10 -0

+		ctx->dev = dev;
+		ctx->dsi[0] = dsi;
+		ctx->dsi[1] = secondary;
+
+		ret = truly_wqxga_panel_add(ctx);
+		if (ret) {
+			put_device(&secondary->dev);
+			return ret;
+		}
+	}

This should go when we use the of-graph bindings. The second mipi_dsi_device needs to be created manually in the driver using mipi_dsi_device_register_full(). You could look at the ADV7511 bridge driver as an example. We would eventually need to call mipi_dsi_attach() on the second device too.

Thanks,
Archit

Quoted 33 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +32 -0

+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		if (secondary)
+			truly_wqxga_panel_del(ctx);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
+{
+	struct truly_wqxga *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+
+	/* delete panel only for the DSI1 interface */
+	if (ctx)
+		truly_wqxga_panel_del(ctx);
+
+	return 0;
+}
+
+static const struct of_device_id truly_wqxga_of_match[] = {
+	{ .compatible = "truly,dual_wqxga", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, truly_wqxga_of_match);
+
+static struct mipi_dsi_driver truly_wqxga_driver = {
+	.driver = {

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 530

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +10 -0

+		.name = "panel_truly_wqxga",
+		.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 wqxga DSI Panel Driver");
+MODULE_LICENSE("GPL v2");

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

[email protected]

2018-04-10 02:30 UTC #9 lore raw

Quoted 22 lines · Archit Taneja

Hi Abhinav,
Thanks for posting this driver. Some comments below.
Quoted 17 lines · Abhinav Kumar
From: Archit Taneja <[email protected]>
Add support for truly dual DSI video mode panel
panel used in MSM reference platforms >
Signed-off-by Archit Taneja <[email protected]>
Signed-off-by Abhinav Kumar <[email protected]>
---
  .../bindings/display/truly,dual_wqxga.txt          |  47 ++
  drivers/gpu/drm/panel/Kconfig                      |   7 +
  drivers/gpu/drm/panel/Makefile                     |   1 +
  drivers/gpu/drm/panel/panel-truly-dual-dsi.c       | 530 
+++++++++++++++++++++
  4 files changed, 585 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/truly,dual_wqxga.txt
create mode 100644 drivers/gpu/drm/panel/panel-truly-dual-dsi.c

Quoted 15 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +4 -0
new file mode 100644
index 0000000..a1b24c1
@@ -0,0 +1,47 @@
+Truly model NT35597 1440x2560 DSI Panel
+
+Required properties:
+- compatible: should be "truly,dual_wqxga"
The compatible string, kernel config and the driver file should be based on the panel model no. There can be many truly based panels that support wqxga. Something like "truly,nt35597" would be better.

[Abhinav] Yes, will change it in v2.

Quoted 10 lines

Quoted 9 lines
Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +1 -0
+- vdda-supply: phandle of the regulator that provides the supply 
voltage
+ Power IC supply
+- lab-supply: phandle of the regulator that provides the supply voltage
+ for LCD bias
+- ibb-supply: phandle of the regulator that provides the supply voltage
+ for LCD bias

Quoted 4 lines

Both seem to have the same description. Aren't lab and ibb qualcomm specific terms? Could we use the pin names specified in the panel's data sheet?

[Abhinav] There isnt a number specified on the data sheet. I will use vdispp-supply and vdispn-supply . Does that work?

Quoted 10 lines

Quoted 6 lines
+- 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
Could we describe here how to use this gpio? I.e, whether we need to set it to low for dual DSI, etc?

[Abhinav] Yes, will do this in v2.

Quoted 6 lines

Quoted 2 lines
+- display-timings: Node for the Panel timings
+- link2: phandle to the secondary node of the panel
The link2 binding was a temporary hack we used. We should use the of-graph bindings to represent the two DSI input ports of the panel.

[Abhinav] Alright will start using of-graph in the next patchset.

Quoted 38 lines

Documentation/devicetree/bindings/display/truly,dual_wqxga.txt new file +31 -0

+
+Example:
+
+	dsi@ae94000 {
+		panel@0 {
+			compatible = "truly,dual_wqxga";
+			reg = <0>;
+			link2 = <&link2>;
+			vdda-supply = <&pm8998_l14>;
+
+			pinctrl-names = "default", "suspend";
+			pinctrl-0 = <&sde_dsi_active>;
+			pinctrl-1 = <&sde_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>;
+				};
+			};
+		};
+	};

drivers/gpu/drm/panel/Kconfig +0 -0

index 988048e..a63c3f7 100644
@@ -168,4 +168,11 @@ config DRM_PANEL_SITRONIX_ST7789V
  	  Say Y here if you want to enable support for the Sitronix
  	  ST7789V controller for 240x320 LCD panels

Quoted 8 lines

drivers/gpu/drm/panel/Kconfig config DRM_PANEL_SITRONIX_ST7789V · line 178
Quoted 8 lines
drivers/gpu/drm/panel/Kconfig +6 -0
  +config DRM_PANEL_TRULY_WQXGA
+	tristate "Truly WQXGA"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	help
+	  Say Y here if you want to enable support for Truly WQXGA Dual DSI
+	  Video Mode panel
  endmenu

[Abhinav] These configs will be updated too

Quoted 13 lines

Quoted 13 lines
drivers/gpu/drm/panel/Makefile +0 -0
index 3d2a88d..64891f6 100644
@@ -17,3 +17,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
drivers/gpu/drm/panel/Makefile +1 -0
+obj-$(CONFIG_DRM_PANEL_TRULY_WQXGA) += panel-truly-dual-dsi.o
drivers/gpu/drm/panel/panel-truly-dual-dsi.c +0 -0

Quoted 9 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 1
Quoted 6 lines
new file mode 100644
index 0000000..47891ee
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +1 -0
@@ -0,0 +1,530 @@
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
We should use the SPDX license headers here.

[Abhinav] Is there a reference I can use? Need to make sure of this.

Quoted 60 lines

Quoted 60 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +2 -0
+ *
+ * This program is free software; you can redistribute it and/or 
modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +32 -0
+
+#include <linux/gpio/consumer.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>
+
+struct truly_wqxga {
+	struct device *dev;
+	struct drm_panel panel;
+
+	struct regulator_bulk_data supplies[4];
+
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *mode_gpio;
+
+	struct backlight_device *backlight;
+	struct videomode vm;
+
+	struct mipi_dsi_device *dsi[2];
+
+	bool prepared;
+	bool enabled;
+};
+
+static inline struct truly_wqxga *panel_to_truly_wqxga(struct 
drm_panel *panel)
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +15 -0
+{
+	return container_of(panel, struct truly_wqxga, panel);
+}
+
+static int truly_wqxga_power_on(struct truly_wqxga *ctx)
+{
+	int ret;
+
+	ret = regulator_set_load(ctx->supplies[1].consumer, 62000);
+	if (ret)
+		return ret;
+
+	ret = regulator_set_load(ctx->supplies[2].consumer, 100000);
+	if (ret)
+		return ret;

Quoted 9 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 63
Quoted 4 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +4 -0
+
+	ret = regulator_set_load(ctx->supplies[3].consumer, 100000);
+	if (ret)
+		return ret;
We could make a const static struct array specifying the regulator names (to be used during probe) and their corresponding power on and power off loads. That should make things a bit cleaner.

[Abhinav] Yes, will do it in the next patchset.

Quoted 226 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +2 -0

+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),

ctx->supplies);

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +27 -0

+	if (ret < 0)
+		return ret;
+
+	msleep(20);
+	gpiod_set_value(ctx->reset_gpio, 1);
+	usleep_range(20000, 20100);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(20000, 20100);
+	gpiod_set_value(ctx->reset_gpio, 1);
+	usleep_range(50000, 50100);
+
+	/* dual port */
+	gpiod_set_value(ctx->mode_gpio, 0);
+
+	return 0;
+}
+
+static int truly_wqxga_power_off(struct truly_wqxga *ctx)
+{
+	int ret;
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+	ret = regulator_set_load(ctx->supplies[1].consumer, 62000);
+	if (ret)
+		return ret;
+
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies),

ctx->supplies);

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +195 -0

+}
+
+static int truly_wqxga_disable(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_POWERDOWN;
+		backlight_update_status(ctx->backlight);
+	}
+
+	ctx->enabled = false;
+	return 0;
+}
+
+static int truly_wqxga_unprepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	struct mipi_dsi_device **dsis = ctx->dsi;
+	int ret = 0, i;
+
+	if (!ctx->prepared)
+		return 0;
+
+	dsis[0]->mode_flags = 0;
+	dsis[1]->mode_flags = 0;
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_set_display_off(dsis[i]) < 0)
+			ret = -ECOMM;
+	msleep(78);
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_enter_sleep_mode(dsis[i]) < 0)
+			ret = -ECOMM;
+	msleep(78);
+
+	truly_wqxga_power_off(ctx);
+
+	ctx->prepared = false;
+	return ret;
+}
+
+#define MAX_LEN	5
+struct {
+	u8 commands[MAX_LEN];
+	int size;
+} panel_cmds[] = { /* CMD2_P0 */
+		   { { 0xff, 0x20 }, 2 },
+		   { { 0xfb, 0x01 }, 2 },
+		   { { 0x00, 0x01 }, 2 },
+		   { { 0x01, 0x55 }, 2 },
+		   { { 0x02, 0x45 }, 2 },
+		   { { 0x05, 0x40 }, 2 },
+		   { { 0x06, 0x19 }, 2 },
+		   { { 0x07, 0x1e }, 2 },
+		   { { 0x0b, 0x73 }, 2 },
+		   { { 0x0c, 0x73 }, 2 },
+		   { { 0x0e, 0xb0 }, 2 },
+		   { { 0x0f, 0xae }, 2 },
+		   { { 0x11, 0xb8 }, 2 },
+		   { { 0x13, 0x00 }, 2 },
+		   { { 0x58, 0x80 }, 2 },
+		   { { 0x59, 0x01 }, 2 },
+		   { { 0x5a, 0x00 }, 2 },
+		   { { 0x5b, 0x01 }, 2 },
+		   { { 0x5c, 0x80 }, 2 },
+		   { { 0x5d, 0x81 }, 2 },
+		   { { 0x5e, 0x00 }, 2 },
+		   { { 0x5f, 0x01 }, 2 },
+		   { { 0x72, 0x11 }, 2 },
+		   { { 0x68, 0x03 }, 2 },
+		   /* CMD2_P4 */
+		   { { 0xFF, 0x24 }, 2 },
+		   { { 0xFB, 0x01 }, 2 },
+		   { { 0x00, 0x1C }, 2 },
+		   { { 0x01, 0x0B }, 2 },
+		   { { 0x02, 0x0C }, 2 },
+		   { { 0x03, 0x01 }, 2 },
+		   { { 0x04, 0x0F }, 2 },
+		   { { 0x05, 0x10 }, 2 },
+		   { { 0x06, 0x10 }, 2 },
+		   { { 0x07, 0x10 }, 2 },
+		   { { 0x08, 0x89 }, 2 },
+		   { { 0x09, 0x8A }, 2 },
+		   { { 0x0A, 0x13 }, 2 },
+		   { { 0x0B, 0x13 }, 2 },
+		   { { 0x0C, 0x15 }, 2 },
+		   { { 0x0D, 0x15 }, 2 },
+		   { { 0x0E, 0x17 }, 2 },
+		   { { 0x0F, 0x17 }, 2 },
+		   { { 0x10, 0x1C }, 2 },
+		   { { 0x11, 0x0B }, 2 },
+		   { { 0x12, 0x0C }, 2 },
+		   { { 0x13, 0x01 }, 2 },
+		   { { 0x14, 0x0F }, 2 },
+		   { { 0x15, 0x10 }, 2 },
+		   { { 0x16, 0x10 }, 2 },
+		   { { 0x17, 0x10 }, 2 },
+		   { { 0x18, 0x89 }, 2 },
+		   { { 0x19, 0x8A }, 2 },
+		   { { 0x1A, 0x13 }, 2 },
+		   { { 0x1B, 0x13 }, 2 },
+		   { { 0x1C, 0x15 }, 2 },
+		   { { 0x1D, 0x15 }, 2 },
+		   { { 0x1E, 0x17 }, 2 },
+		   { { 0x1F, 0x17 }, 2 },
+		   /* STV */
+		   { { 0x20, 0x40 }, 2 },
+		   { { 0x21, 0x01 }, 2 },
+		   { { 0x22, 0x00 }, 2 },
+		   { { 0x23, 0x40 }, 2 },
+		   { { 0x24, 0x40 }, 2 },
+		   { { 0x25, 0x6D }, 2 },
+		   { { 0x26, 0x40 }, 2 },
+		   { { 0x27, 0x40 }, 2 },
+		   /* Vend */
+		   { { 0xE0, 0x00 }, 2 },
+		   { { 0xDC, 0x21 }, 2 },
+		   { { 0xDD, 0x22 }, 2 },
+		   { { 0xDE, 0x07 }, 2 },
+		   { { 0xDF, 0x07 }, 2 },
+		   { { 0xE3, 0x6D }, 2 },
+		   { { 0xE1, 0x07 }, 2 },
+		   { { 0xE2, 0x07 }, 2 },
+		   /* UD */
+		   { { 0x29, 0xD8 }, 2 },
+		   { { 0x2A, 0x2A }, 2 },
+		   /* CLK */
+		   { { 0x4B, 0x03 }, 2 },
+		   { { 0x4C, 0x11 }, 2 },
+		   { { 0x4D, 0x10 }, 2 },
+		   { { 0x4E, 0x01 }, 2 },
+		   { { 0x4F, 0x01 }, 2 },
+		   { { 0x50, 0x10 }, 2 },
+		   { { 0x51, 0x00 }, 2 },
+		   { { 0x52, 0x80 }, 2 },
+		   { { 0x53, 0x00 }, 2 },
+		   { { 0x56, 0x00 }, 2 },
+		   { { 0x54, 0x07 }, 2 },
+		   { { 0x58, 0x07 }, 2 },
+		   { { 0x55, 0x25 }, 2 },
+		   /* Reset XDONB */
+		   { { 0x5B, 0x43 }, 2 },
+		   { { 0x5C, 0x00 }, 2 },
+		   { { 0x5F, 0x73 }, 2 },
+		   { { 0x60, 0x73 }, 2 },
+		   { { 0x63, 0x22 }, 2 },
+		   { { 0x64, 0x00 }, 2 },
+		   { { 0x67, 0x08 }, 2 },
+		   { { 0x68, 0x04 }, 2 },
+		   /* Resolution:1440x2560 */
+		   { { 0x72, 0x02 }, 2 },
+		   /* mux */
+		   { { 0x7A, 0x80 }, 2 },
+		   { { 0x7B, 0x91 }, 2 },
+		   { { 0x7C, 0xD8 }, 2 },
+		   { { 0x7D, 0x60 }, 2 },
+		   { { 0x7F, 0x15 }, 2 },
+		   { { 0x75, 0x15 }, 2 },
+		   /* ABOFF */
+		   { { 0xB3, 0xC0 }, 2 },
+		   { { 0xB4, 0x00 }, 2 },
+		   { { 0xB5, 0x00 }, 2 },
+		   /* Source EQ */
+		   { { 0x78, 0x00 }, 2 },
+		   { { 0x79, 0x00 }, 2 },
+		   { { 0x80, 0x00 }, 2 },
+		   { { 0x83, 0x00 }, 2 },
+		   /* FP BP */
+		   { { 0x93, 0x0A }, 2 },
+		   { { 0x94, 0x0A }, 2 },
+		   /* Inversion Type */
+		   { { 0x8A, 0x00 }, 2 },
+		   { { 0x9B, 0xFF }, 2 },
+		   /* IMGSWAP =1 @PortSwap=1 */
+		   { { 0x9D, 0xB0 }, 2 },
+		   { { 0x9F, 0x63 }, 2 },
+		   { { 0x98, 0x10 }, 2 },
+		   /* FRM */
+		   { { 0xEC, 0x00 }, 2 },
+		   /* CMD1 */
+		   { { 0xFF, 0x10 }, 2 },
+		    /* VBP+VSA=,VFP = 10H */
+		   { { 0x3B, 0x03, 0x0A, 0x0A, }, 4 },
+		   /* FTE on */
+		   { { 0x35, 0x00 }, 2 },
+		   /* EN_BK =1(auto black) */
+		   { { 0xE5, 0x01 }, 2 },
+		   /* CMD mode(10) VDO mode(03) */
+		   { { 0xBB, 0x03 }, 2 },
+		   /* Non Reload MTP */

Quoted 8 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 291
Quoted 4 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +4 -0
+		   { { 0xFB, 0x01 }, 2 },
+		   /* SlpOut + DispOn */
+		   //05 01 00 00 78 00 02 11 00
+		   //05 01 00 00 78 00 02 29 00
We can drop the commented lines. If possible, it would be nice to have some more description about the registers, but I can imagine that info being hard to retrieve.

[Abhinav] Yes, will remove the commented lines. Yes will be hard to give more info on the regs.

Quoted 25 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +24 -0

+};
+
+static int truly_wqxga_prepare(struct drm_panel *panel)
+{
+	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
+	struct mipi_dsi_device **dsis = ctx->dsi;
+	struct mipi_dsi_device *d;
+	int ret, i, j;
+
+	if (ctx->prepared)
+		return 0;
+
+	ret = truly_wqxga_power_on(ctx);
+	if (ret < 0)
+		return ret;
+
+	dsis[0]->mode_flags |= MIPI_DSI_MODE_LPM;
+	dsis[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	for (j = 0; j < ARRAY_SIZE(panel_cmds); j++) {
+		for (i = 0; i < 2; i++) {
+			d = dsis[i];
+			ret = mipi_dsi_dcs_write_buffer(dsis[i],
+					panel_cmds[j].commands,

Quoted 8 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 320
Quoted 5 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +5 -0
+					panel_cmds[j].size);
+			if (ret < 0) {
+				/* continue anyway */
+				dev_err(ctx->dev, "failed to cmd no %d, err: %d\n",
+						j, ret);
This may have been done for debug purposes. It would be probably better to bail out.

[Abhinav] Ok, will bail out here.

Quoted 177 lines

Quoted 160 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +140 -0

+			}
+		}
+	}
+
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_exit_sleep_mode(dsis[i]) < 0) {
+			dev_err(ctx->dev, "failed to exit sleep mode\n");
+			return -ECOMM;
+		}
+	msleep(78);
+
+	for (i = 0; i < 2; i++)
+		if (mipi_dsi_dcs_set_display_on(dsis[i]) < 0) {
+			dev_err(ctx->dev, "failed to send display on\n");
+			return -ECOMM;
+		}
+	msleep(78);
+
+	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) {
+		dev_err(ctx->dev, "failed to create a new display mode\n");
+		return 0;
+	}
+
+	drm_display_mode_from_videomode(&ctx->vm, mode);
+	connector->display_info.width_mm = 74;
+	connector->display_info.height_mm = 131;
+	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;
+
+	ctx->supplies[0].supply = "vddio";
+	ctx->supplies[1].supply = "vdda";
+	ctx->supplies[2].supply = "lab";
+	ctx->supplies[3].supply = "ibb";
+	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)) {
+		dev_err(dev, "cannot get reset-gpios %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)) {
+		dev_err(dev, "cannot get mode gpio %ld\n",
+			PTR_ERR(ctx->mode_gpio));
+		ctx->mode_gpio = NULL;
+		return PTR_ERR(ctx->mode_gpio);
+	}
+
+	ret = pinctrl_pm_select_default_state(dev);
+	if (ret) {
+		dev_err(dev, "%s: failed to set pinctrl default state, %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = of_get_videomode(dev->of_node, &ctx->vm, 0);
+	if (ret < 0)
+		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 void truly_wqxga_panel_del(struct truly_wqxga *ctx)
+{
+	if (ctx->panel.dev)
+		drm_panel_remove(&ctx->panel);
+
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	if (ctx->dsi[1])
+		put_device(&ctx->dsi[1]->dev);
+}
+
+static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct truly_wqxga *ctx;
+	struct mipi_dsi_device *secondary = NULL;
+	struct device_node *np;
+	int ret;
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |

MIPI_DSI_CLOCK_NON_CONTINUOUS |

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +18 -0

+			  MIPI_DSI_MODE_LPM;
+
+	/* Only DSI-LINK1 node has "link2" entry. */
+	np = of_parse_phandle(dsi->dev.of_node, "link2", 0);
+	if (np) {
+		secondary = of_find_mipi_dsi_device_by_node(np);
+		of_node_put(np);
+
+		if (!secondary)
+			return -EPROBE_DEFER;
+
+		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+		if (!ctx) {
+			put_device(&secondary->dev);
+			return -ENOMEM;
+		}
+		mipi_dsi_set_drvdata(dsi, ctx);
+

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 488

Quoted 10 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +10 -0

+		ctx->dev = dev;
+		ctx->dsi[0] = dsi;
+		ctx->dsi[1] = secondary;
+
+		ret = truly_wqxga_panel_add(ctx);
+		if (ret) {
+			put_device(&secondary->dev);
+			return ret;
+		}
+	}

Quoted 3 lines

Thanks,
Archit

[Abhinav] I dont think we should follow bridge's example. In our DT, the panel nodes are listed as child nodes.
The mipi_dsi_host_register API adds all the children of using of_mipi_dsi_device_add. This calls a mipi_dsi_device_register_full(). twice because the panel is listed as a child on both the DSIs.

In bridge, the bridge driver is not listed as a child
of the DSI node, and hence an explicit call to mipi_dsi_device_register_full() is required.

Hence, yes I will use of-graph bindings and get rid of "link2" but the part to register the panel only for the master controller and allocate ctx only then seems required to me.

Also, an explicit call to mipi_dsi_device_register_full() doesnt seem necessary.

Can you please comment whether our understanding is aligned with yours here?

Quoted 39 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +39 -0

+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		if (secondary)
+			truly_wqxga_panel_del(ctx);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
+{
+	struct truly_wqxga *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+
+	/* delete panel only for the DSI1 interface */
+	if (ctx)
+		truly_wqxga_panel_del(ctx);
+
+	return 0;
+}
+
+static const struct of_device_id truly_wqxga_of_match[] = {
+	{ .compatible = "truly,dual_wqxga", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, truly_wqxga_of_match);
+
+static struct mipi_dsi_driver truly_wqxga_driver = {
+	.driver = {
+		.name = "panel_truly_wqxga",
+		.of_match_table = truly_wqxga_of_match,
+	},
+	.probe = truly_wqxga_probe,
+	.remove = truly_wqxga_remove,
+};
+module_mipi_dsi_driver(truly_wqxga_driver);

Quoted 8 lines

drivers/gpu/drm/panel/panel-truly-dual-dsi.c line 530
Quoted 4 lines
drivers/gpu/drm/panel/panel-truly-dual-dsi.c new file +3 -0
+
+MODULE_DESCRIPTION("truly wqxga DSI Panel Driver");
+MODULE_LICENSE("GPL v2");
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel