[PATCH 0/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[PATCH 0/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Guido Günther]

The panel uses a Focaltech FT8006p, the touch part is handled by the already
existing edt-ft5x06. It can be found in e.g. the Librem 5.

This series is against next-20200814.

Guido Günther (3):
  dt-bindings: vendor-prefixes: Add mantix vendor prefix
  dt-bindings: Add Mantix MLAF057WE51-X panel bindings
  drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

 .../display/panel/mantix,mlaf057we51-x.yaml   |  73 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 362 ++++++++++++++++++
 6 files changed, 456 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c

-- 
2.26.2

[PATCH 1/3] dt-bindings: vendor-prefixes: Add mantix vendor prefix

[Guido Günther]

Add prefix for Mantix Display Technology Co.,Ltd.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2baee2c817c1a..59d4c8b068c4d 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -611,6 +611,8 @@ patternProperties:
     description: Linux Automation GmbH
   "^macnica,.*":
     description: Macnica Americas
+  "^mantix,.*":
+    description: Mantix Display Technology Co.,Ltd.
   "^mapleboard,.*":
     description: Mapleboard.org
   "^marvell,.*":
-- 
2.26.2

[PATCH 2/3] dt-bindings: Add Mantix MLAF057WE51-X panel bindings

[Guido Günther]

The panel uses a Focaltech FT8006p, the touch part is handled by the
already existing edt-ft5x06.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 .../display/panel/mantix,mlaf057we51-x.yaml   | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
new file mode 100644
index 0000000000000..349f3380ac940
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/mantix,mlaf057we51-x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mantix MLAF057WE51-X 5.7" 720x1440 TFT LCD panel
+
+maintainers:
+  - Guido Günther <agx@sigxcpu.org>
+
+description: |
+             Mantix MLAF057WE51 X is a 720x1440 TFT LCD panel
+             connected using a MIPI-DSI video interface.
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - mantix,mlaf057we51-x
+
+  port: true
+  reg:
+    maxItems: 1
+    description: DSI virtual channel
+
+  avdd-supply:
+    description: Positive analog power supply
+
+  avee-supply:
+    description: Negative analog power supply
+
+  vddi-supply:
+    description: 1.8V I/O voltage supply
+
+  reset-gpios:
+    description: GPIO used for the reset pin
+    maxItems: 1
+
+  backlight:
+    description: Backlight used by the panel
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - avee-supply
+  - vddi-supply
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    dsi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      panel@0 {
+        compatible = "mantix,mlaf057we51-x";
+        reg = <0>;
+        avdd-supply = <&reg_avdd>;
+        avee-supply = <&reg_avee>;
+        vddi-supply = <&reg_1v8_p>;
+        reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
+        backlight = <&backlight>;
+      };
+    };
+...
-- 
2.26.2

[PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Guido Günther]

The panel uses a Focaltech FT8006p, the touch part is handled by the
already existing edt-ft5x06.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 362 ++++++++++++++++++
 4 files changed, 381 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 83ba7b62651f7..7dfe4cc3d4ec8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5474,6 +5474,13 @@ S:	Maintained
 F:	drivers/gpu/drm/panel/panel-lvds.c
 F:	Documentation/devicetree/bindings/display/panel/lvds.yaml
 
+DRM DRIVER FOR MANTIX MLAF057WE51 PANELS
+M:	Guido Günther <agx@sigxcpu.org>
+R:	Purism Kernel Team <kernel@puri.sm>
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
+F:	drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
+
 DRM DRIVER FOR MATROX G200/G400 GRAPHICS CARDS
 S:	Orphan / Obsolete
 F:	drivers/gpu/drm/mga/
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index de2f2a452be55..8d97d07c58713 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -217,6 +217,17 @@ config DRM_PANEL_NOVATEK_NT39016
 	  Say Y here if you want to enable support for the panels built
 	  around the Novatek NT39016 display controller.
 
+config DRM_PANEL_MANTIX_MLAF057WE51
+	tristate "Mantix MLAF057WE51-X MIPI-DSI LCD panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for the Mantix
+	  MLAF057WE51-X MIPI DSI panel as e.g. used in the Librem 5. It
+	  has a resolution of 720x1440 pixels, a built in backlight and touch
+	  controller.
+
 config DRM_PANEL_OLIMEX_LCD_OLINUXINO
 	tristate "Olimex LCD-OLinuXino panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index e45ceac6286fd..15a4e77529514 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
+obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
 obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
 obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
 obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o
diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
new file mode 100644
index 0000000000000..6c07bcdb75937
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mantix MLAF057WE51 5.7" MIPI-DSI panel driver
+ *
+ * Copyright (C) Purism SPC 2020
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/display_timing.h>
+#include <video/mipi_display.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+#define DRV_NAME "panel-mantix-mlaf057we51"
+
+/* Manufacturer specific Commands send via DSI */
+#define MANTIX_CMD_OTP_STOP_RELOAD_MIPI 0x41
+#define MANTIX_CMD_INT_CANCEL           0x4C
+
+struct mantix {
+	struct device *dev;
+	struct drm_panel panel;
+	struct gpio_desc *reset_gpio;
+
+	struct regulator *avdd;
+	struct regulator *avee;
+	struct regulator *vddi;
+};
+
+static inline struct mantix *panel_to_mantix(struct drm_panel *panel)
+{
+	return container_of(panel, struct mantix, panel);
+}
+
+#define dsi_generic_write_seq(dsi, seq...) do {				\
+		static const u8 d[] = { seq };				\
+		int ret;						\
+		ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));	\
+		if (ret < 0)						\
+			return ret;					\
+	} while (0)
+
+static int mantix_init_sequence(struct mantix *ctx)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	struct device *dev = ctx->dev;
+
+	/*
+	 * Init sequence was supplied by the panel vendor.
+	 */
+	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A);
+
+	dsi_generic_write_seq(dsi, MANTIX_CMD_INT_CANCEL, 0x03);
+	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A, 0x03);
+	dsi_generic_write_seq(dsi, 0x80, 0xA9, 0x00);
+
+	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A, 0x09);
+	dsi_generic_write_seq(dsi, 0x80, 0x64, 0x00, 0x64, 0x00, 0x00);
+	msleep(20);
+
+	DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n");
+	return 0;
+}
+
+static int mantix_enable(struct drm_panel *panel)
+{
+	struct mantix *ctx = panel_to_mantix(panel);
+	struct device *dev = ctx->dev;
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
+	int ret;
+
+	ret = mantix_init_sequence(ctx);
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n",
+			      ret);
+		return ret;
+	}
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n");
+		return ret;
+	}
+	msleep(20);
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret)
+		return ret;
+	usleep_range(10000, 12000);
+
+	ret = mipi_dsi_turn_on_peripheral(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to turn on peripheral\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mantix_disable(struct drm_panel *panel)
+{
+	struct mantix *ctx = panel_to_mantix(panel);
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	int ret;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0)
+		DRM_DEV_ERROR(ctx->dev,
+			      "Failed to turn off the display: %d\n", ret);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0)
+		DRM_DEV_ERROR(ctx->dev,
+			      "Failed to enter sleep mode: %d\n", ret);
+
+	mipi_dsi_dcs_enter_sleep_mode(dsi);
+	return 0;
+}
+
+static int mantix_unprepare(struct drm_panel *panel)
+{
+	struct mantix *ctx = panel_to_mantix(panel);
+
+	regulator_disable(ctx->avdd);
+	regulator_disable(ctx->avee);
+	regulator_disable(ctx->vddi);
+
+	return 0;
+}
+
+static int mantix_prepare(struct drm_panel *panel)
+{
+	struct mantix *ctx = panel_to_mantix(panel);
+	int ret;
+
+	/* Focaltech FT8006P, section 7.3.1 and 7.3.4 */
+	DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
+	ret = regulator_enable(ctx->vddi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			      "Failed to enable vddi supply: %d\n", ret);
+		return ret;
+	}
+	/* T1 + T2 */
+	usleep_range(8000, 10000);
+
+	ret = regulator_enable(ctx->avdd);
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			      "Failed to enable avdd supply: %d\n", ret);
+		return ret;
+	}
+
+	/* T2d */
+	usleep_range(3500, 4000);
+	ret = regulator_enable(ctx->avee);
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev,
+			      "Failed to enable avee supply: %d\n", ret);
+		return ret;
+	}
+
+	/* T3+T5 */
+	usleep_range(10000, 12000);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	usleep_range(5150, 7000);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+
+	/* T6 */
+	msleep(50);
+
+	return 0;
+}
+
+static const struct drm_display_mode default_mode = {
+	.hdisplay    = 720,
+	.hsync_start = 720 + 45,
+	.hsync_end   = 720 + 45 + 14,
+	.htotal	     = 720 + 45 + 14 + 25,
+	.vdisplay    = 1440,
+	.vsync_start = 1440 + 130,
+	.vsync_end   = 1440 + 130 + 8,
+	.vtotal	     = 1440 + 130 + 8 + 106,
+	.clock	     = 85298,
+	.flags	     = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+	.width_mm    = 65,
+	.height_mm   = 130,
+};
+
+static int mantix_get_modes(struct drm_panel *panel,
+			    struct drm_connector *connector)
+{
+	struct mantix *ctx = panel_to_mantix(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(connector->dev, &default_mode);
+	if (!mode) {
+		DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u\n",
+			      default_mode.hdisplay, default_mode.vdisplay,
+			      drm_mode_vrefresh(mode));
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs mantix_drm_funcs = {
+	.disable   = mantix_disable,
+	.unprepare = mantix_unprepare,
+	.prepare   = mantix_prepare,
+	.enable	   = mantix_enable,
+	.get_modes = mantix_get_modes,
+};
+
+static int mantix_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct mantix *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset_gpio)) {
+		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
+		return PTR_ERR(ctx->reset_gpio);
+	}
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+	ctx->dev = dev;
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
+		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+
+	ctx->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(ctx->avdd)) {
+		ret = PTR_ERR(ctx->avdd);
+		if (ret != -EPROBE_DEFER)
+			DRM_DEV_ERROR(dev,
+				      "Failed to request avdd regulator: %d\n",
+				      ret);
+		return ret;
+	}
+	ctx->avee = devm_regulator_get(dev, "avee");
+	if (IS_ERR(ctx->avee)) {
+		ret = PTR_ERR(ctx->avee);
+		if (ret != -EPROBE_DEFER)
+			DRM_DEV_ERROR(dev,
+				      "Failed to request avee regulator: %d\n",
+				      ret);
+		return ret;
+	}
+	ctx->vddi = devm_regulator_get(dev, "vddi");
+	if (IS_ERR(ctx->vddi)) {
+		ret = PTR_ERR(ctx->vddi);
+		if (ret != -EPROBE_DEFER)
+			DRM_DEV_ERROR(dev,
+				      "Failed to request vddi regulator: %d\n",
+				      ret);
+		return ret;
+	}
+
+	drm_panel_init(&ctx->panel, dev, &mantix_drm_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	ret = drm_panel_of_backlight(&ctx->panel);
+	if (ret)
+		return ret;
+	drm_panel_add(&ctx->panel);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev,
+			      "mipi_dsi_attach failed (%d). Is host ready?\n",
+			      ret);
+		drm_panel_remove(&ctx->panel);
+		return ret;
+	}
+
+	DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n",
+		     default_mode.hdisplay, default_mode.vdisplay,
+		     drm_mode_vrefresh(&default_mode),
+		     mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
+
+	return 0;
+}
+
+static void mantix_shutdown(struct mipi_dsi_device *dsi)
+{
+	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
+	int ret;
+
+	ret = drm_panel_unprepare(&ctx->panel);
+	if (ret < 0)
+		DRM_DEV_ERROR(&dsi->dev, "Failed to unprepare panel: %d\n",
+			      ret);
+
+	ret = drm_panel_disable(&ctx->panel);
+	if (ret < 0)
+		DRM_DEV_ERROR(&dsi->dev, "Failed to disable panel: %d\n",
+			      ret);
+}
+
+static int mantix_remove(struct mipi_dsi_device *dsi)
+{
+	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
+	int ret;
+
+	mantix_shutdown(dsi);
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		DRM_DEV_ERROR(&dsi->dev, "Failed to detach from DSI host: %d\n",
+			      ret);
+
+	drm_panel_remove(&ctx->panel);
+
+	return 0;
+}
+
+static const struct of_device_id mantix_of_match[] = {
+	{ .compatible = "mantix,mlaf057we51-x" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mantix_of_match);
+
+static struct mipi_dsi_driver mantix_driver = {
+	.probe	= mantix_probe,
+	.remove = mantix_remove,
+	.shutdown = mantix_shutdown,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = mantix_of_match,
+	},
+};
+module_mipi_dsi_driver(mantix_driver);
+
+MODULE_AUTHOR("Guido Günther <agx@sigxcpu.org>");
+MODULE_DESCRIPTION("DRM driver for Mantix MLAF057WE51-X MIPI DSI panel");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2

Re: [PATCH 2/3] dt-bindings: Add Mantix MLAF057WE51-X panel bindings

[Sam Ravnborg]

Hi Guido.

On Fri, Aug 14, 2020 at 03:36:22PM +0200, Guido Günther wrote:
> The panel uses a Focaltech FT8006p, the touch part is handled by the
> already existing edt-ft5x06.
> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>

A few trivialities.

> ---
>  .../display/panel/mantix,mlaf057we51-x.yaml   | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> new file mode 100644
> index 0000000000000..349f3380ac940
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/mantix,mlaf057we51-x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mantix MLAF057WE51-X 5.7" 720x1440 TFT LCD panel
> +
> +maintainers:
> +  - Guido Günther <agx@sigxcpu.org>
> +
> +description: |
> +             Mantix MLAF057WE51 X is a 720x1440 TFT LCD panel
> +             connected using a MIPI-DSI video interface.
Indent text with two spaces only.
And I have learned that '|' is only needed to preserve formatting - so
it can be dropped.

> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mantix,mlaf057we51-x
This is a list - so needs an extra 2 spaces indent.
See https://lore.kernel.org/linux-devicetree/f1963eb9-283f-e903-2a3a-4f324d71d418@lucaceresoli.net/T/#m65900317fb948f6c40e8fb521f2201fba3c301a7
for examples where Rob fixes this.

> +
> +  port: true
> +  reg:
> +    maxItems: 1
> +    description: DSI virtual channel
> +
> +  avdd-supply:
> +    description: Positive analog power supply
> +
> +  avee-supply:
> +    description: Negative analog power supply
> +
> +  vddi-supply:
> +    description: 1.8V I/O voltage supply
> +
> +  reset-gpios:
> +    description: GPIO used for the reset pin
> +    maxItems: 1
Use reset-gpios: true as we already have it in panel-common.yaml

> +
> +  backlight:
> +    description: Backlight used by the panel
> +    $ref: "/schemas/types.yaml#/definitions/phandle"
Use backlight from panel-common.yaml.

> +
> +required:
> +  - compatible
> +  - reg
> +  - avdd-supply
> +  - avee-supply
> +  - vddi-supply
> +  - reset-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    dsi {
My personal preference is indent with 4 spaces in examples but there are
no rules so feel free to ignore.
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      panel@0 {
> +        compatible = "mantix,mlaf057we51-x";
> +        reg = <0>;
> +        avdd-supply = <&reg_avdd>;
> +        avee-supply = <&reg_avee>;
> +        vddi-supply = <&reg_1v8_p>;
> +        reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
> +        backlight = <&backlight>;
> +      };
> +    };
I think we need an ampty line here.
> +...
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Sam Ravnborg]

Hi Guido.

On Fri, Aug 14, 2020 at 03:36:23PM +0200, Guido Günther wrote:
> The panel uses a Focaltech FT8006p, the touch part is handled by the
> already existing edt-ft5x06.
> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>

Looks good.
A few notes in the following, nothing major.

	Sam

> ---
>  MAINTAINERS                                   |   7 +
>  drivers/gpu/drm/panel/Kconfig                 |  11 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 362 ++++++++++++++++++
>  4 files changed, 381 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83ba7b62651f7..7dfe4cc3d4ec8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5474,6 +5474,13 @@ S:	Maintained
>  F:	drivers/gpu/drm/panel/panel-lvds.c
>  F:	Documentation/devicetree/bindings/display/panel/lvds.yaml
>  
> +DRM DRIVER FOR MANTIX MLAF057WE51 PANELS
> +M:	Guido Günther <agx@sigxcpu.org>
> +R:	Purism Kernel Team <kernel@puri.sm>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> +F:	drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> +
>  DRM DRIVER FOR MATROX G200/G400 GRAPHICS CARDS
>  S:	Orphan / Obsolete
>  F:	drivers/gpu/drm/mga/
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index de2f2a452be55..8d97d07c58713 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -217,6 +217,17 @@ config DRM_PANEL_NOVATEK_NT39016
>  	  Say Y here if you want to enable support for the panels built
>  	  around the Novatek NT39016 display controller.
>  
> +config DRM_PANEL_MANTIX_MLAF057WE51
> +	tristate "Mantix MLAF057WE51-X MIPI-DSI LCD panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y here if you want to enable support for the Mantix
> +	  MLAF057WE51-X MIPI DSI panel as e.g. used in the Librem 5. It
> +	  has a resolution of 720x1440 pixels, a built in backlight and touch
> +	  controller.
> +
>  config DRM_PANEL_OLIMEX_LCD_OLINUXINO
>  	tristate "Olimex LCD-OLinuXino panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index e45ceac6286fd..15a4e77529514 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>  obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
>  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
>  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
> +obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
>  obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
>  obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
>  obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o
> diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> new file mode 100644
> index 0000000000000..6c07bcdb75937
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mantix MLAF057WE51 5.7" MIPI-DSI panel driver
> + *
> + * Copyright (C) Purism SPC 2020
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/media-bus-format.h>
Include not needed.

> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/display_timing.h>
I do not think this include is needed

> +#include <video/mipi_display.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +
> +#define DRV_NAME "panel-mantix-mlaf057we51"
> +
> +/* Manufacturer specific Commands send via DSI */
> +#define MANTIX_CMD_OTP_STOP_RELOAD_MIPI 0x41
> +#define MANTIX_CMD_INT_CANCEL           0x4C
> +
> +struct mantix {
> +	struct device *dev;
> +	struct drm_panel panel;
> +	struct gpio_desc *reset_gpio;
> +
> +	struct regulator *avdd;
> +	struct regulator *avee;
> +	struct regulator *vddi;
> +};
> +
> +static inline struct mantix *panel_to_mantix(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct mantix, panel);
> +}
> +
> +#define dsi_generic_write_seq(dsi, seq...) do {				\
> +		static const u8 d[] = { seq };				\
> +		int ret;						\
> +		ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));	\
> +		if (ret < 0)						\
> +			return ret;					\
> +	} while (0)
> +
> +static int mantix_init_sequence(struct mantix *ctx)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	struct device *dev = ctx->dev;
> +
> +	/*
> +	 * Init sequence was supplied by the panel vendor.
> +	 */
> +	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A);
> +
> +	dsi_generic_write_seq(dsi, MANTIX_CMD_INT_CANCEL, 0x03);
> +	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A, 0x03);
> +	dsi_generic_write_seq(dsi, 0x80, 0xA9, 0x00);
> +
> +	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A, 0x09);
> +	dsi_generic_write_seq(dsi, 0x80, 0x64, 0x00, 0x64, 0x00, 0x00);
> +	msleep(20);
> +
> +	DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n");
> +	return 0;
> +}
> +
> +static int mantix_enable(struct drm_panel *panel)
> +{
> +	struct mantix *ctx = panel_to_mantix(panel);
> +	struct device *dev = ctx->dev;
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
> +	int ret;
> +
> +	ret = mantix_init_sequence(ctx);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n",
> +			      ret);
> +		return ret;
> +	}
> +
> +	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n");
> +		return ret;
> +	}
> +	msleep(20);
> +
> +	ret = mipi_dsi_dcs_set_display_on(dsi);
> +	if (ret)
> +		return ret;
> +	usleep_range(10000, 12000);
> +
> +	ret = mipi_dsi_turn_on_peripheral(dsi);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev, "Failed to turn on peripheral\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mantix_disable(struct drm_panel *panel)
> +{
> +	struct mantix *ctx = panel_to_mantix(panel);
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_set_display_off(dsi);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(ctx->dev,
> +			      "Failed to turn off the display: %d\n", ret);
> +
> +	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(ctx->dev,
> +			      "Failed to enter sleep mode: %d\n", ret);
> +
> +	mipi_dsi_dcs_enter_sleep_mode(dsi);
Does the display really need to enter sleep mode twice?

> +	return 0;
> +}
> +
> +static int mantix_unprepare(struct drm_panel *panel)
> +{
> +	struct mantix *ctx = panel_to_mantix(panel);
> +
> +	regulator_disable(ctx->avdd);
> +	regulator_disable(ctx->avee);
> +	regulator_disable(ctx->vddi);

The order the regulators are disabled is not the reverse of the order
thay are enabled. Unless this is on purpose it would be more logical and
less confusing if unprepare is the reverse of prepare.


> +
> +	return 0;
> +}
> +
> +static int mantix_prepare(struct drm_panel *panel)
> +{
> +	struct mantix *ctx = panel_to_mantix(panel);
> +	int ret;
> +
> +	/* Focaltech FT8006P, section 7.3.1 and 7.3.4 */
> +	DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
> +	ret = regulator_enable(ctx->vddi);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(ctx->dev,
> +			      "Failed to enable vddi supply: %d\n", ret);
> +		return ret;
> +	}
> +	/* T1 + T2 */
> +	usleep_range(8000, 10000);
> +
> +	ret = regulator_enable(ctx->avdd);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(ctx->dev,
> +			      "Failed to enable avdd supply: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* T2d */
> +	usleep_range(3500, 4000);
> +	ret = regulator_enable(ctx->avee);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(ctx->dev,
> +			      "Failed to enable avee supply: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* T3+T5 */
> +	usleep_range(10000, 12000);
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(5150, 7000);
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +
> +	/* T6 */
> +	msleep(50);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> +	.hdisplay    = 720,
> +	.hsync_start = 720 + 45,
> +	.hsync_end   = 720 + 45 + 14,
> +	.htotal	     = 720 + 45 + 14 + 25,
> +	.vdisplay    = 1440,
> +	.vsync_start = 1440 + 130,
> +	.vsync_end   = 1440 + 130 + 8,
> +	.vtotal	     = 1440 + 130 + 8 + 106,
> +	.clock	     = 85298,
> +	.flags	     = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +	.width_mm    = 65,
> +	.height_mm   = 130,
> +};
> +
> +static int mantix_get_modes(struct drm_panel *panel,
> +			    struct drm_connector *connector)
> +{
> +	struct mantix *ctx = panel_to_mantix(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(connector->dev, &default_mode);
> +	if (!mode) {
> +		DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u\n",
> +			      default_mode.hdisplay, default_mode.vdisplay,
> +			      drm_mode_vrefresh(mode));
> +		return -ENOMEM;
> +	}
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	connector->display_info.width_mm = mode->width_mm;
> +	connector->display_info.height_mm = mode->height_mm;
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs mantix_drm_funcs = {
> +	.disable   = mantix_disable,
> +	.unprepare = mantix_unprepare,
> +	.prepare   = mantix_prepare,
> +	.enable	   = mantix_enable,
> +	.get_modes = mantix_get_modes,
> +};
> +
> +static int mantix_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct mantix *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> +		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +
> +	ctx->avdd = devm_regulator_get(dev, "avdd");
> +	if (IS_ERR(ctx->avdd)) {
> +		ret = PTR_ERR(ctx->avdd);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev,
> +				      "Failed to request avdd regulator: %d\n",
> +				      ret);
In these modern times linelegth of 100 is acceptable, so avoid
linewrapping here and likewise below.

> +		return ret;
> +	}
> +	ctx->avee = devm_regulator_get(dev, "avee");
> +	if (IS_ERR(ctx->avee)) {
> +		ret = PTR_ERR(ctx->avee);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev,
> +				      "Failed to request avee regulator: %d\n",
> +				      ret);
> +		return ret;
> +	}
> +	ctx->vddi = devm_regulator_get(dev, "vddi");
> +	if (IS_ERR(ctx->vddi)) {
> +		ret = PTR_ERR(ctx->vddi);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev,
> +				      "Failed to request vddi regulator: %d\n",
> +				      ret);
> +		return ret;
> +	}
> +
> +	drm_panel_init(&ctx->panel, dev, &mantix_drm_funcs,
> +		       DRM_MODE_CONNECTOR_DSI);
> +
> +	ret = drm_panel_of_backlight(&ctx->panel);
> +	if (ret)
> +		return ret;
empty line?
> +	drm_panel_add(&ctx->panel);
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev,
> +			      "mipi_dsi_attach failed (%d). Is host ready?\n",
> +			      ret);
> +		drm_panel_remove(&ctx->panel);
> +		return ret;
> +	}
> +
> +	DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n",
> +		     default_mode.hdisplay, default_mode.vdisplay,
> +		     drm_mode_vrefresh(&default_mode),
> +		     mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
> +
> +	return 0;
> +}
> +
> +static void mantix_shutdown(struct mipi_dsi_device *dsi)
> +{
> +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> +	int ret;
> +
> +	ret = drm_panel_unprepare(&ctx->panel);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "Failed to unprepare panel: %d\n",
> +			      ret);
> +
> +	ret = drm_panel_disable(&ctx->panel);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "Failed to disable panel: %d\n",
> +			      ret);
> +}
In shutdown we usually ignore any errors.

> +
> +static int mantix_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> +	int ret;
> +
> +	mantix_shutdown(dsi);
> +
> +	ret = mipi_dsi_detach(dsi);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "Failed to detach from DSI host: %d\n",
> +			      ret);
Likewise in remove we ignore any errors.

> +
> +	drm_panel_remove(&ctx->panel);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mantix_of_match[] = {
> +	{ .compatible = "mantix,mlaf057we51-x" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mantix_of_match);
> +
> +static struct mipi_dsi_driver mantix_driver = {
> +	.probe	= mantix_probe,
> +	.remove = mantix_remove,
> +	.shutdown = mantix_shutdown,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.of_match_table = mantix_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(mantix_driver);
> +
> +MODULE_AUTHOR("Guido Günther <agx@sigxcpu.org>");
> +MODULE_DESCRIPTION("DRM driver for Mantix MLAF057WE51-X MIPI DSI panel");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Sam Ravnborg]

Hi Guido.

> +static int mantix_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct mantix *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> +		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +
> +	ctx->avdd = devm_regulator_get(dev, "avdd");
> +	if (IS_ERR(ctx->avdd)) {
> +		ret = PTR_ERR(ctx->avdd);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev,
> +				      "Failed to request avdd regulator: %d\n",
> +				      ret);
> +		return ret;
> +	}

Consider to use the recently added dev_err_probe() here and below.
Note: Not part of drm-misc-next yet - but hopefully after -rc1
when a backmerge is done.

	Sam

> +	ctx->avee = devm_regulator_get(dev, "avee");
> +	if (IS_ERR(ctx->avee)) {
> +		ret = PTR_ERR(ctx->avee);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev,
> +				      "Failed to request avee regulator: %d\n",
> +				      ret);
> +		return ret;
> +	}
> +	ctx->vddi = devm_regulator_get(dev, "vddi");
> +	if (IS_ERR(ctx->vddi)) {
> +		ret = PTR_ERR(ctx->vddi);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_DEV_ERROR(dev,
> +				      "Failed to request vddi regulator: %d\n",
> +				      ret);
> +		return ret;
> +	}
> +
> +	drm_panel_init(&ctx->panel, dev, &mantix_drm_funcs,
> +		       DRM_MODE_CONNECTOR_DSI);
> +
> +	ret = drm_panel_of_backlight(&ctx->panel);
> +	if (ret)
> +		return ret;
> +	drm_panel_add(&ctx->panel);
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev,
> +			      "mipi_dsi_attach failed (%d). Is host ready?\n",
> +			      ret);
> +		drm_panel_remove(&ctx->panel);
> +		return ret;
> +	}
> +
> +	DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n",
> +		     default_mode.hdisplay, default_mode.vdisplay,
> +		     drm_mode_vrefresh(&default_mode),
> +		     mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
> +
> +	return 0;
> +}
> +
> +static void mantix_shutdown(struct mipi_dsi_device *dsi)
> +{
> +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> +	int ret;
> +
> +	ret = drm_panel_unprepare(&ctx->panel);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "Failed to unprepare panel: %d\n",
> +			      ret);
> +
> +	ret = drm_panel_disable(&ctx->panel);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "Failed to disable panel: %d\n",
> +			      ret);
> +}
> +
> +static int mantix_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> +	int ret;
> +
> +	mantix_shutdown(dsi);
> +
> +	ret = mipi_dsi_detach(dsi);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "Failed to detach from DSI host: %d\n",
> +			      ret);
> +
> +	drm_panel_remove(&ctx->panel);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mantix_of_match[] = {
> +	{ .compatible = "mantix,mlaf057we51-x" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mantix_of_match);
> +
> +static struct mipi_dsi_driver mantix_driver = {
> +	.probe	= mantix_probe,
> +	.remove = mantix_remove,
> +	.shutdown = mantix_shutdown,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.of_match_table = mantix_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(mantix_driver);
> +
> +MODULE_AUTHOR("Guido Günther <agx@sigxcpu.org>");
> +MODULE_DESCRIPTION("DRM driver for Mantix MLAF057WE51-X MIPI DSI panel");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Guido Günther]

Hi Sam,
On Sat, Aug 15, 2020 at 12:02:30PM +0200, Sam Ravnborg wrote:
> Hi Guido.
> 
> > +static int mantix_probe(struct mipi_dsi_device *dsi)
> > +{
> > +	struct device *dev = &dsi->dev;
> > +	struct mantix *ctx;
> > +	int ret;
> > +
> > +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > +	if (IS_ERR(ctx->reset_gpio)) {
> > +		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
> > +		return PTR_ERR(ctx->reset_gpio);
> > +	}
> > +
> > +	mipi_dsi_set_drvdata(dsi, ctx);
> > +	ctx->dev = dev;
> > +
> > +	dsi->lanes = 4;
> > +	dsi->format = MIPI_DSI_FMT_RGB888;
> > +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > +		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > +
> > +	ctx->avdd = devm_regulator_get(dev, "avdd");
> > +	if (IS_ERR(ctx->avdd)) {
> > +		ret = PTR_ERR(ctx->avdd);
> > +		if (ret != -EPROBE_DEFER)
> > +			DRM_DEV_ERROR(dev,
> > +				      "Failed to request avdd regulator: %d\n",
> > +				      ret);
> > +		return ret;
> > +	}
> 
> Consider to use the recently added dev_err_probe() here and below.
> Note: Not part of drm-misc-next yet - but hopefully after -rc1
> when a backmerge is done.

In fact I did decided against it since i was told that missing dev_* and
DRM_* logging shouldn't be done. So is that o.k. nowadays?
Cheers,
 -- Guido

> 
> 	Sam
> 
> > +	ctx->avee = devm_regulator_get(dev, "avee");
> > +	if (IS_ERR(ctx->avee)) {
> > +		ret = PTR_ERR(ctx->avee);
> > +		if (ret != -EPROBE_DEFER)
> > +			DRM_DEV_ERROR(dev,
> > +				      "Failed to request avee regulator: %d\n",
> > +				      ret);
> > +		return ret;
> > +	}
> > +	ctx->vddi = devm_regulator_get(dev, "vddi");
> > +	if (IS_ERR(ctx->vddi)) {
> > +		ret = PTR_ERR(ctx->vddi);
> > +		if (ret != -EPROBE_DEFER)
> > +			DRM_DEV_ERROR(dev,
> > +				      "Failed to request vddi regulator: %d\n",
> > +				      ret);
> > +		return ret;
> > +	}
> > +
> > +	drm_panel_init(&ctx->panel, dev, &mantix_drm_funcs,
> > +		       DRM_MODE_CONNECTOR_DSI);
> > +
> > +	ret = drm_panel_of_backlight(&ctx->panel);
> > +	if (ret)
> > +		return ret;
> > +	drm_panel_add(&ctx->panel);
> > +
> > +	ret = mipi_dsi_attach(dsi);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(dev,
> > +			      "mipi_dsi_attach failed (%d). Is host ready?\n",
> > +			      ret);
> > +		drm_panel_remove(&ctx->panel);
> > +		return ret;
> > +	}
> > +
> > +	DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n",
> > +		     default_mode.hdisplay, default_mode.vdisplay,
> > +		     drm_mode_vrefresh(&default_mode),
> > +		     mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
> > +
> > +	return 0;
> > +}
> > +
> > +static void mantix_shutdown(struct mipi_dsi_device *dsi)
> > +{
> > +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> > +	int ret;
> > +
> > +	ret = drm_panel_unprepare(&ctx->panel);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(&dsi->dev, "Failed to unprepare panel: %d\n",
> > +			      ret);
> > +
> > +	ret = drm_panel_disable(&ctx->panel);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(&dsi->dev, "Failed to disable panel: %d\n",
> > +			      ret);
> > +}
> > +
> > +static int mantix_remove(struct mipi_dsi_device *dsi)
> > +{
> > +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> > +	int ret;
> > +
> > +	mantix_shutdown(dsi);
> > +
> > +	ret = mipi_dsi_detach(dsi);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(&dsi->dev, "Failed to detach from DSI host: %d\n",
> > +			      ret);
> > +
> > +	drm_panel_remove(&ctx->panel);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id mantix_of_match[] = {
> > +	{ .compatible = "mantix,mlaf057we51-x" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, mantix_of_match);
> > +
> > +static struct mipi_dsi_driver mantix_driver = {
> > +	.probe	= mantix_probe,
> > +	.remove = mantix_remove,
> > +	.shutdown = mantix_shutdown,
> > +	.driver = {
> > +		.name = DRV_NAME,
> > +		.of_match_table = mantix_of_match,
> > +	},
> > +};
> > +module_mipi_dsi_driver(mantix_driver);
> > +
> > +MODULE_AUTHOR("Guido Günther <agx@sigxcpu.org>");
> > +MODULE_DESCRIPTION("DRM driver for Mantix MLAF057WE51-X MIPI DSI panel");
> > +MODULE_LICENSE("GPL v2");
> > -- 
> > 2.26.2
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Sam Ravnborg]

On Sat, Aug 15, 2020 at 12:40:22PM +0200, Guido Günther wrote:
> Hi Sam,
> On Sat, Aug 15, 2020 at 12:02:30PM +0200, Sam Ravnborg wrote:
> > Hi Guido.
> > 
> > > +static int mantix_probe(struct mipi_dsi_device *dsi)
> > > +{
> > > +	struct device *dev = &dsi->dev;
> > > +	struct mantix *ctx;
> > > +	int ret;
> > > +
> > > +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > > +	if (!ctx)
> > > +		return -ENOMEM;
> > > +
> > > +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(ctx->reset_gpio)) {
> > > +		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
> > > +		return PTR_ERR(ctx->reset_gpio);
> > > +	}
> > > +
> > > +	mipi_dsi_set_drvdata(dsi, ctx);
> > > +	ctx->dev = dev;
> > > +
> > > +	dsi->lanes = 4;
> > > +	dsi->format = MIPI_DSI_FMT_RGB888;
> > > +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > > +		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > > +
> > > +	ctx->avdd = devm_regulator_get(dev, "avdd");
> > > +	if (IS_ERR(ctx->avdd)) {
> > > +		ret = PTR_ERR(ctx->avdd);
> > > +		if (ret != -EPROBE_DEFER)
> > > +			DRM_DEV_ERROR(dev,
> > > +				      "Failed to request avdd regulator: %d\n",
> > > +				      ret);
> > > +		return ret;
> > > +	}
> > 
> > Consider to use the recently added dev_err_probe() here and below.
> > Note: Not part of drm-misc-next yet - but hopefully after -rc1
> > when a backmerge is done.
> 
> In fact I did decided against it since i was told that missing dev_* and
> DRM_* logging shouldn't be done. So is that o.k. nowadays?
s/missing/mixing/

I often request that logging is consistent - so I recognize the
argument.

For panel/* I have not made up my mind what I think is the best
approach. The DRM_DEV_* and DRM_* logging do not add much value.
So I have been tempted several times to convert all logging in
panel/ to dev_* and pr_* (when no struct device * is available).
That would also avoid that we mix up logging.

We have drm_* logging - but they require a valid drm_device * which we
do not have in the the panel drivers. So they are ruled out here.

Do you have any opinions/comments on this?

	Sam

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Sam Ravnborg]

Hi Guido.

> > In fact I did decided against it since i was told that missing dev_* and
> > DRM_* logging shouldn't be done. So is that o.k. nowadays?
> s/missing/mixing/
> 
> I often request that logging is consistent - so I recognize the
> argument.
> 
> For panel/* I have not made up my mind what I think is the best
> approach. The DRM_DEV_* and DRM_* logging do not add much value.
> So I have been tempted several times to convert all logging in
> panel/ to dev_* and pr_* (when no struct device * is available).
> That would also avoid that we mix up logging.
> 
> We have drm_* logging - but they require a valid drm_device * which we
> do not have in the the panel drivers. So they are ruled out here.
> 
> Do you have any opinions/comments on this?
I went ahead and dropped the DRM_ logging in drm/panel, you are copied
on the patchset. Feedback on the patches would be nice.

	Sam

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Guido Günther]

Hi,
On Sat, Aug 15, 2020 at 12:46:51PM +0200, Sam Ravnborg wrote:
> On Sat, Aug 15, 2020 at 12:40:22PM +0200, Guido Günther wrote:
> > Hi Sam,
> > On Sat, Aug 15, 2020 at 12:02:30PM +0200, Sam Ravnborg wrote:
> > > Hi Guido.
> > > 
> > > > +static int mantix_probe(struct mipi_dsi_device *dsi)
> > > > +{
> > > > +	struct device *dev = &dsi->dev;
> > > > +	struct mantix *ctx;
> > > > +	int ret;
> > > > +
> > > > +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > > > +	if (!ctx)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > > +	if (IS_ERR(ctx->reset_gpio)) {
> > > > +		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
> > > > +		return PTR_ERR(ctx->reset_gpio);
> > > > +	}
> > > > +
> > > > +	mipi_dsi_set_drvdata(dsi, ctx);
> > > > +	ctx->dev = dev;
> > > > +
> > > > +	dsi->lanes = 4;
> > > > +	dsi->format = MIPI_DSI_FMT_RGB888;
> > > > +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > > > +		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > > > +
> > > > +	ctx->avdd = devm_regulator_get(dev, "avdd");
> > > > +	if (IS_ERR(ctx->avdd)) {
> > > > +		ret = PTR_ERR(ctx->avdd);
> > > > +		if (ret != -EPROBE_DEFER)
> > > > +			DRM_DEV_ERROR(dev,
> > > > +				      "Failed to request avdd regulator: %d\n",
> > > > +				      ret);
> > > > +		return ret;
> > > > +	}
> > > 
> > > Consider to use the recently added dev_err_probe() here and below.
> > > Note: Not part of drm-misc-next yet - but hopefully after -rc1
> > > when a backmerge is done.
> > 
> > In fact I did decided against it since i was told that missing dev_* and
> > DRM_* logging shouldn't be done. So is that o.k. nowadays?
> s/missing/mixing/
> 
> I often request that logging is consistent - so I recognize the
> argument.
> 
> For panel/* I have not made up my mind what I think is the best
> approach. The DRM_DEV_* and DRM_* logging do not add much value.
> So I have been tempted several times to convert all logging in
> panel/ to dev_* and pr_* (when no struct device * is available).
> That would also avoid that we mix up logging.
> 
> We have drm_* logging - but they require a valid drm_device * which we
> do not have in the the panel drivers. So they are ruled out here.
> 
> Do you have any opinions/comments on this?

I think for panel drivers DRM_* does not give any bonus so moving to
{dev,pr}_* sounds good. I just wonder if other drm parts don't need
`dev_drm_err_probe()` (or similar) anyway. But then maybe dyn_debug
is enough nowadays to not need DRM_DEV_DEBUG_* either?
Cheers,
 -- Guido

> 
> 	Sam
> 

Re: [PATCH 2/3] dt-bindings: Add Mantix MLAF057WE51-X panel bindings

[Guido Günther]

Hi Sam,
On Sat, Aug 15, 2020 at 10:39:17AM +0200, Sam Ravnborg wrote:
> Hi Guido.
> 
> On Fri, Aug 14, 2020 at 03:36:22PM +0200, Guido Günther wrote:
> > The panel uses a Focaltech FT8006p, the touch part is handled by the
> > already existing edt-ft5x06.
> > 
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> 
> A few trivialities.

Thanks for having a look. One remark inline:

> 
> > ---
> >  .../display/panel/mantix,mlaf057we51-x.yaml   | 73 +++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > new file mode 100644
> > index 0000000000000..349f3380ac940
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/mantix,mlaf057we51-x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Mantix MLAF057WE51-X 5.7" 720x1440 TFT LCD panel
> > +
> > +maintainers:
> > +  - Guido Günther <agx@sigxcpu.org>
> > +
> > +description: |
> > +             Mantix MLAF057WE51 X is a 720x1440 TFT LCD panel
> > +             connected using a MIPI-DSI video interface.
> Indent text with two spaces only.
> And I have learned that '|' is only needed to preserve formatting - so
> it can be dropped.
> 
> > +
> > +allOf:
> > +  - $ref: panel-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - mantix,mlaf057we51-x
> This is a list - so needs an extra 2 spaces indent.
> See https://lore.kernel.org/linux-devicetree/f1963eb9-283f-e903-2a3a-4f324d71d418@lucaceresoli.net/T/#m65900317fb948f6c40e8fb521f2201fba3c301a7
> for examples where Rob fixes this.

Doesn't this only apply if the 'outer element' is a list too so e.g.:

   - enum
     - foo

trips up yamllint but

   enum
     - foo

doesn't. Since yamllint was happy i kept it as is (looking at your
reference suggests that too).

All the rest made sense and i fixed that for the upcoming v2.
Thanks for having a look!
 -- Guido

> 
> > +
> > +  port: true
> > +  reg:
> > +    maxItems: 1
> > +    description: DSI virtual channel
> > +
> > +  avdd-supply:
> > +    description: Positive analog power supply
> > +
> > +  avee-supply:
> > +    description: Negative analog power supply
> > +
> > +  vddi-supply:
> > +    description: 1.8V I/O voltage supply
> > +
> > +  reset-gpios:
> > +    description: GPIO used for the reset pin
> > +    maxItems: 1
> Use reset-gpios: true as we already have it in panel-common.yaml
> 
> > +
> > +  backlight:
> > +    description: Backlight used by the panel
> > +    $ref: "/schemas/types.yaml#/definitions/phandle"
> Use backlight from panel-common.yaml.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - avdd-supply
> > +  - avee-supply
> > +  - vddi-supply
> > +  - reset-gpios
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    dsi {
> My personal preference is indent with 4 spaces in examples but there are
> no rules so feel free to ignore.
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      panel@0 {
> > +        compatible = "mantix,mlaf057we51-x";
> > +        reg = <0>;
> > +        avdd-supply = <&reg_avdd>;
> > +        avee-supply = <&reg_avee>;
> > +        vddi-supply = <&reg_1v8_p>;
> > +        reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
> > +        backlight = <&backlight>;
> > +      };
> > +    };
> I think we need an ampty line here.
> > +...
> > -- 
> > 2.26.2
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH 2/3] dt-bindings: Add Mantix MLAF057WE51-X panel bindings

[Sam Ravnborg]

Hi Guido.

On Sat, Aug 15, 2020 at 06:47:37PM +0200, Guido Günther wrote:
> Hi Sam,
> On Sat, Aug 15, 2020 at 10:39:17AM +0200, Sam Ravnborg wrote:
> > Hi Guido.
> > 
> > On Fri, Aug 14, 2020 at 03:36:22PM +0200, Guido Günther wrote:
> > > The panel uses a Focaltech FT8006p, the touch part is handled by the
> > > already existing edt-ft5x06.
> > > 
> > > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> > 
> > A few trivialities.
> 
> Thanks for having a look. One remark inline:
> 
> > 
> > > ---
> > >  .../display/panel/mantix,mlaf057we51-x.yaml   | 73 +++++++++++++++++++
> > >  1 file changed, 73 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > > new file mode 100644
> > > index 0000000000000..349f3380ac940
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > > @@ -0,0 +1,73 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/display/panel/mantix,mlaf057we51-x.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Mantix MLAF057WE51-X 5.7" 720x1440 TFT LCD panel
> > > +
> > > +maintainers:
> > > +  - Guido Günther <agx@sigxcpu.org>
> > > +
> > > +description: |
> > > +             Mantix MLAF057WE51 X is a 720x1440 TFT LCD panel
> > > +             connected using a MIPI-DSI video interface.
> > Indent text with two spaces only.
> > And I have learned that '|' is only needed to preserve formatting - so
> > it can be dropped.
> > 
> > > +
> > > +allOf:
> > > +  - $ref: panel-common.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - mantix,mlaf057we51-x
> > This is a list - so needs an extra 2 spaces indent.
> > See https://lore.kernel.org/linux-devicetree/f1963eb9-283f-e903-2a3a-4f324d71d418@lucaceresoli.net/T/#m65900317fb948f6c40e8fb521f2201fba3c301a7
> > for examples where Rob fixes this.
> 
> Doesn't this only apply if the 'outer element' is a list too so e.g.:
> 
>    - enum
>      - foo
> 
> trips up yamllint but
> 
>    enum
>      - foo
> 
> doesn't. Since yamllint was happy i kept it as is (looking at your
> reference suggests that too).

You are right, I missed that this was not a list (no '-' in front of
enum).
I would not be able to do this right without tool assistance.

	Sam

> 
> All the rest made sense and i fixed that for the upcoming v2.
> Thanks for having a look!
>  -- Guido
> 
> > 
> > > +
> > > +  port: true
> > > +  reg:
> > > +    maxItems: 1
> > > +    description: DSI virtual channel
> > > +
> > > +  avdd-supply:
> > > +    description: Positive analog power supply
> > > +
> > > +  avee-supply:
> > > +    description: Negative analog power supply
> > > +
> > > +  vddi-supply:
> > > +    description: 1.8V I/O voltage supply
> > > +
> > > +  reset-gpios:
> > > +    description: GPIO used for the reset pin
> > > +    maxItems: 1
> > Use reset-gpios: true as we already have it in panel-common.yaml
> > 
> > > +
> > > +  backlight:
> > > +    description: Backlight used by the panel
> > > +    $ref: "/schemas/types.yaml#/definitions/phandle"
> > Use backlight from panel-common.yaml.
> > 
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - avdd-supply
> > > +  - avee-supply
> > > +  - vddi-supply
> > > +  - reset-gpios
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/gpio/gpio.h>
> > > +
> > > +    dsi {
> > My personal preference is indent with 4 spaces in examples but there are
> > no rules so feel free to ignore.
> > > +      #address-cells = <1>;
> > > +      #size-cells = <0>;
> > > +      panel@0 {
> > > +        compatible = "mantix,mlaf057we51-x";
> > > +        reg = <0>;
> > > +        avdd-supply = <&reg_avdd>;
> > > +        avee-supply = <&reg_avee>;
> > > +        vddi-supply = <&reg_1v8_p>;
> > > +        reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
> > > +        backlight = <&backlight>;
> > > +      };
> > > +    };
> > I think we need an ampty line here.
> > > +...
> > > -- 
> > > 2.26.2
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 

Re: [PATCH 3/3] drm/panel: Add panel driver for the Mantix MLAF057WE51-X DSI panel

[Guido Günther]

Hi Sam,
On Sat, Aug 15, 2020 at 11:32:26AM +0200, Sam Ravnborg wrote:
> Hi Guido.
> 
> On Fri, Aug 14, 2020 at 03:36:23PM +0200, Guido Günther wrote:
> > The panel uses a Focaltech FT8006p, the touch part is handled by the
> > already existing edt-ft5x06.
> > 
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> 
> Looks good.
> A few notes in the following, nothing major.

Thanks. I've added your suggestions to v2.
 -- Guido
> 
> 	Sam
> 
> > ---
> >  MAINTAINERS                                   |   7 +
> >  drivers/gpu/drm/panel/Kconfig                 |  11 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 362 ++++++++++++++++++
> >  4 files changed, 381 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 83ba7b62651f7..7dfe4cc3d4ec8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5474,6 +5474,13 @@ S:	Maintained
> >  F:	drivers/gpu/drm/panel/panel-lvds.c
> >  F:	Documentation/devicetree/bindings/display/panel/lvds.yaml
> >  
> > +DRM DRIVER FOR MANTIX MLAF057WE51 PANELS
> > +M:	Guido Günther <agx@sigxcpu.org>
> > +R:	Purism Kernel Team <kernel@puri.sm>
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
> > +F:	drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> > +
> >  DRM DRIVER FOR MATROX G200/G400 GRAPHICS CARDS
> >  S:	Orphan / Obsolete
> >  F:	drivers/gpu/drm/mga/
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index de2f2a452be55..8d97d07c58713 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -217,6 +217,17 @@ config DRM_PANEL_NOVATEK_NT39016
> >  	  Say Y here if you want to enable support for the panels built
> >  	  around the Novatek NT39016 display controller.
> >  
> > +config DRM_PANEL_MANTIX_MLAF057WE51
> > +	tristate "Mantix MLAF057WE51-X MIPI-DSI LCD panel"
> > +	depends on OF
> > +	depends on DRM_MIPI_DSI
> > +	depends on BACKLIGHT_CLASS_DEVICE
> > +	help
> > +	  Say Y here if you want to enable support for the Mantix
> > +	  MLAF057WE51-X MIPI DSI panel as e.g. used in the Librem 5. It
> > +	  has a resolution of 720x1440 pixels, a built in backlight and touch
> > +	  controller.
> > +
> >  config DRM_PANEL_OLIMEX_LCD_OLINUXINO
> >  	tristate "Olimex LCD-OLinuXino panel"
> >  	depends on OF
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index e45ceac6286fd..15a4e77529514 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> >  obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
> >  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
> >  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
> > +obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
> >  obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
> >  obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
> >  obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o
> > diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> > new file mode 100644
> > index 0000000000000..6c07bcdb75937
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
> > @@ -0,0 +1,362 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Mantix MLAF057WE51 5.7" MIPI-DSI panel driver
> > + *
> > + * Copyright (C) Purism SPC 2020
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/media-bus-format.h>
> Include not needed.
> 
> > +#include <linux/module.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include <video/display_timing.h>
> I do not think this include is needed
> 
> > +#include <video/mipi_display.h>
> > +
> > +#include <drm/drm_mipi_dsi.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_print.h>
> > +
> > +#define DRV_NAME "panel-mantix-mlaf057we51"
> > +
> > +/* Manufacturer specific Commands send via DSI */
> > +#define MANTIX_CMD_OTP_STOP_RELOAD_MIPI 0x41
> > +#define MANTIX_CMD_INT_CANCEL           0x4C
> > +
> > +struct mantix {
> > +	struct device *dev;
> > +	struct drm_panel panel;
> > +	struct gpio_desc *reset_gpio;
> > +
> > +	struct regulator *avdd;
> > +	struct regulator *avee;
> > +	struct regulator *vddi;
> > +};
> > +
> > +static inline struct mantix *panel_to_mantix(struct drm_panel *panel)
> > +{
> > +	return container_of(panel, struct mantix, panel);
> > +}
> > +
> > +#define dsi_generic_write_seq(dsi, seq...) do {				\
> > +		static const u8 d[] = { seq };				\
> > +		int ret;						\
> > +		ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));	\
> > +		if (ret < 0)						\
> > +			return ret;					\
> > +	} while (0)
> > +
> > +static int mantix_init_sequence(struct mantix *ctx)
> > +{
> > +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > +	struct device *dev = ctx->dev;
> > +
> > +	/*
> > +	 * Init sequence was supplied by the panel vendor.
> > +	 */
> > +	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A);
> > +
> > +	dsi_generic_write_seq(dsi, MANTIX_CMD_INT_CANCEL, 0x03);
> > +	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A, 0x03);
> > +	dsi_generic_write_seq(dsi, 0x80, 0xA9, 0x00);
> > +
> > +	dsi_generic_write_seq(dsi, MANTIX_CMD_OTP_STOP_RELOAD_MIPI, 0x5A, 0x09);
> > +	dsi_generic_write_seq(dsi, 0x80, 0x64, 0x00, 0x64, 0x00, 0x00);
> > +	msleep(20);
> > +
> > +	DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n");
> > +	return 0;
> > +}
> > +
> > +static int mantix_enable(struct drm_panel *panel)
> > +{
> > +	struct mantix *ctx = panel_to_mantix(panel);
> > +	struct device *dev = ctx->dev;
> > +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
> > +	int ret;
> > +
> > +	ret = mantix_init_sequence(ctx);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n",
> > +			      ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n");
> > +		return ret;
> > +	}
> > +	msleep(20);
> > +
> > +	ret = mipi_dsi_dcs_set_display_on(dsi);
> > +	if (ret)
> > +		return ret;
> > +	usleep_range(10000, 12000);
> > +
> > +	ret = mipi_dsi_turn_on_peripheral(dsi);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(dev, "Failed to turn on peripheral\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mantix_disable(struct drm_panel *panel)
> > +{
> > +	struct mantix *ctx = panel_to_mantix(panel);
> > +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > +	int ret;
> > +
> > +	ret = mipi_dsi_dcs_set_display_off(dsi);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(ctx->dev,
> > +			      "Failed to turn off the display: %d\n", ret);
> > +
> > +	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(ctx->dev,
> > +			      "Failed to enter sleep mode: %d\n", ret);
> > +
> > +	mipi_dsi_dcs_enter_sleep_mode(dsi);
> Does the display really need to enter sleep mode twice?
> 
> > +	return 0;
> > +}
> > +
> > +static int mantix_unprepare(struct drm_panel *panel)
> > +{
> > +	struct mantix *ctx = panel_to_mantix(panel);
> > +
> > +	regulator_disable(ctx->avdd);
> > +	regulator_disable(ctx->avee);
> > +	regulator_disable(ctx->vddi);
> 
> The order the regulators are disabled is not the reverse of the order
> thay are enabled. Unless this is on purpose it would be more logical and
> less confusing if unprepare is the reverse of prepare.
> 
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int mantix_prepare(struct drm_panel *panel)
> > +{
> > +	struct mantix *ctx = panel_to_mantix(panel);
> > +	int ret;
> > +
> > +	/* Focaltech FT8006P, section 7.3.1 and 7.3.4 */
> > +	DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
> > +	ret = regulator_enable(ctx->vddi);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(ctx->dev,
> > +			      "Failed to enable vddi supply: %d\n", ret);
> > +		return ret;
> > +	}
> > +	/* T1 + T2 */
> > +	usleep_range(8000, 10000);
> > +
> > +	ret = regulator_enable(ctx->avdd);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(ctx->dev,
> > +			      "Failed to enable avdd supply: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* T2d */
> > +	usleep_range(3500, 4000);
> > +	ret = regulator_enable(ctx->avee);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(ctx->dev,
> > +			      "Failed to enable avee supply: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* T3+T5 */
> > +	usleep_range(10000, 12000);
> > +
> > +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > +	usleep_range(5150, 7000);
> > +
> > +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> > +
> > +	/* T6 */
> > +	msleep(50);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct drm_display_mode default_mode = {
> > +	.hdisplay    = 720,
> > +	.hsync_start = 720 + 45,
> > +	.hsync_end   = 720 + 45 + 14,
> > +	.htotal	     = 720 + 45 + 14 + 25,
> > +	.vdisplay    = 1440,
> > +	.vsync_start = 1440 + 130,
> > +	.vsync_end   = 1440 + 130 + 8,
> > +	.vtotal	     = 1440 + 130 + 8 + 106,
> > +	.clock	     = 85298,
> > +	.flags	     = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > +	.width_mm    = 65,
> > +	.height_mm   = 130,
> > +};
> > +
> > +static int mantix_get_modes(struct drm_panel *panel,
> > +			    struct drm_connector *connector)
> > +{
> > +	struct mantix *ctx = panel_to_mantix(panel);
> > +	struct drm_display_mode *mode;
> > +
> > +	mode = drm_mode_duplicate(connector->dev, &default_mode);
> > +	if (!mode) {
> > +		DRM_DEV_ERROR(ctx->dev, "Failed to add mode %ux%u@%u\n",
> > +			      default_mode.hdisplay, default_mode.vdisplay,
> > +			      drm_mode_vrefresh(mode));
> > +		return -ENOMEM;
> > +	}
> > +
> > +	drm_mode_set_name(mode);
> > +
> > +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> > +	connector->display_info.width_mm = mode->width_mm;
> > +	connector->display_info.height_mm = mode->height_mm;
> > +	drm_mode_probed_add(connector, mode);
> > +
> > +	return 1;
> > +}
> > +
> > +static const struct drm_panel_funcs mantix_drm_funcs = {
> > +	.disable   = mantix_disable,
> > +	.unprepare = mantix_unprepare,
> > +	.prepare   = mantix_prepare,
> > +	.enable	   = mantix_enable,
> > +	.get_modes = mantix_get_modes,
> > +};
> > +
> > +static int mantix_probe(struct mipi_dsi_device *dsi)
> > +{
> > +	struct device *dev = &dsi->dev;
> > +	struct mantix *ctx;
> > +	int ret;
> > +
> > +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > +	if (IS_ERR(ctx->reset_gpio)) {
> > +		DRM_DEV_ERROR(dev, "cannot get reset gpio\n");
> > +		return PTR_ERR(ctx->reset_gpio);
> > +	}
> > +
> > +	mipi_dsi_set_drvdata(dsi, ctx);
> > +	ctx->dev = dev;
> > +
> > +	dsi->lanes = 4;
> > +	dsi->format = MIPI_DSI_FMT_RGB888;
> > +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > +		MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > +
> > +	ctx->avdd = devm_regulator_get(dev, "avdd");
> > +	if (IS_ERR(ctx->avdd)) {
> > +		ret = PTR_ERR(ctx->avdd);
> > +		if (ret != -EPROBE_DEFER)
> > +			DRM_DEV_ERROR(dev,
> > +				      "Failed to request avdd regulator: %d\n",
> > +				      ret);
> In these modern times linelegth of 100 is acceptable, so avoid
> linewrapping here and likewise below.
> 
> > +		return ret;
> > +	}
> > +	ctx->avee = devm_regulator_get(dev, "avee");
> > +	if (IS_ERR(ctx->avee)) {
> > +		ret = PTR_ERR(ctx->avee);
> > +		if (ret != -EPROBE_DEFER)
> > +			DRM_DEV_ERROR(dev,
> > +				      "Failed to request avee regulator: %d\n",
> > +				      ret);
> > +		return ret;
> > +	}
> > +	ctx->vddi = devm_regulator_get(dev, "vddi");
> > +	if (IS_ERR(ctx->vddi)) {
> > +		ret = PTR_ERR(ctx->vddi);
> > +		if (ret != -EPROBE_DEFER)
> > +			DRM_DEV_ERROR(dev,
> > +				      "Failed to request vddi regulator: %d\n",
> > +				      ret);
> > +		return ret;
> > +	}
> > +
> > +	drm_panel_init(&ctx->panel, dev, &mantix_drm_funcs,
> > +		       DRM_MODE_CONNECTOR_DSI);
> > +
> > +	ret = drm_panel_of_backlight(&ctx->panel);
> > +	if (ret)
> > +		return ret;
> empty line?
> > +	drm_panel_add(&ctx->panel);
> > +
> > +	ret = mipi_dsi_attach(dsi);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(dev,
> > +			      "mipi_dsi_attach failed (%d). Is host ready?\n",
> > +			      ret);
> > +		drm_panel_remove(&ctx->panel);
> > +		return ret;
> > +	}
> > +
> > +	DRM_DEV_INFO(dev, "%ux%u@%u %ubpp dsi %udl - ready\n",
> > +		     default_mode.hdisplay, default_mode.vdisplay,
> > +		     drm_mode_vrefresh(&default_mode),
> > +		     mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes);
> > +
> > +	return 0;
> > +}
> > +
> > +static void mantix_shutdown(struct mipi_dsi_device *dsi)
> > +{
> > +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> > +	int ret;
> > +
> > +	ret = drm_panel_unprepare(&ctx->panel);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(&dsi->dev, "Failed to unprepare panel: %d\n",
> > +			      ret);
> > +
> > +	ret = drm_panel_disable(&ctx->panel);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(&dsi->dev, "Failed to disable panel: %d\n",
> > +			      ret);
> > +}
> In shutdown we usually ignore any errors.
> 
> > +
> > +static int mantix_remove(struct mipi_dsi_device *dsi)
> > +{
> > +	struct mantix *ctx = mipi_dsi_get_drvdata(dsi);
> > +	int ret;
> > +
> > +	mantix_shutdown(dsi);
> > +
> > +	ret = mipi_dsi_detach(dsi);
> > +	if (ret < 0)
> > +		DRM_DEV_ERROR(&dsi->dev, "Failed to detach from DSI host: %d\n",
> > +			      ret);
> Likewise in remove we ignore any errors.
>
> 
> > +
> > +	drm_panel_remove(&ctx->panel);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id mantix_of_match[] = {
> > +	{ .compatible = "mantix,mlaf057we51-x" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, mantix_of_match);
> > +
> > +static struct mipi_dsi_driver mantix_driver = {
> > +	.probe	= mantix_probe,
> > +	.remove = mantix_remove,
> > +	.shutdown = mantix_shutdown,
> > +	.driver = {
> > +		.name = DRV_NAME,
> > +		.of_match_table = mantix_of_match,
> > +	},
> > +};
> > +module_mipi_dsi_driver(mantix_driver);
> > +
> > +MODULE_AUTHOR("Guido Günther <agx@sigxcpu.org>");
> > +MODULE_DESCRIPTION("DRM driver for Mantix MLAF057WE51-X MIPI DSI panel");
> > +MODULE_LICENSE("GPL v2");
> > -- 
> > 2.26.2
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>