[PATCH v3 0/3] drm/panel: add fannal c3004 panel

[PATCH v3 0/3] drm/panel: add fannal c3004 panel

[Paulo Pavacic]

Fannal C3004 is a 2 lane MIPI DSI 480x800 panel which requires initialization with DSI DCS
commands. After few initialization commands delay is required.

Paulo Pavacic (3):
  dt-bindings: add fannal vendor prefix
  dt-bindings: display: panel: add fannal,c3004
  drm/panel-fannal-c3004: Add fannal c3004 DSI panel

 .../bindings/display/panel/fannal,c3004.yaml  |  78 +++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-fannal-c3004.c    | 318 ++++++++++++++++++
 6 files changed, 417 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/fannal,c3004.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-fannal-c3004.c

-- 
2.40.1

[PATCH v3 1/3] dt-bindings: add fannal vendor prefix

[Paulo Pavacic]

Fannal is a one-stop-solution provider for touch displays in industrial
application, committed to delivering a variety of professional high-quality
products and technical services globally.

Website: www.fannal.com

Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
---
 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 82d39ab0231b..f962750f630a 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -462,6 +462,8 @@ patternProperties:
     description: Facebook
   "^fairphone,.*":
     description: Fairphone B.V.
+  "^fannal,.*":
+    description: Fannal Electronics Co., Ltd
   "^faraday,.*":
     description: Faraday Technology Corporation
   "^fastrax,.*":
-- 
2.40.1

[PATCH v3 2/3] dt-bindings: display: panel: add fannal,c3004

[Paulo Pavacic]

Added fannal to vendor-prefixes and dt bindings for Fannal C3004.
Fannal C3004 is a 480x800 MIPI DSI Panel which requires
DCS initialization sequences with certain delays between certain
commands.

Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
---
v4 changelog:
  - add spaces between properties
v3 changelog:
  - renamed yml file
  - refactored yml file to describe fannal,c3004
  - added matrix URI to MAINTAINERS
v2 changelog:
  - revised driver title, now describes purpose
  - revised description, now describes hw
  - revised maintainers, now has only 1 mail
  - removed diacritics from commit/commit author
  - properties/compatible is now enum
  - compatible using only lowercase
  - revised dts example
  - modified MAINTAINERS in this commit (instead of driver commit)
  - dt_bindings_check checked yml
  - checkpatch warning fixed
---
 .../bindings/display/panel/fannal,c3004.yaml  | 78 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/fannal,c3004.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/fannal,c3004.yaml b/Documentation/devicetree/bindings/display/panel/fannal,c3004.yaml
new file mode 100644
index 000000000000..bbddb036094b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/fannal,c3004.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/fannal,c3004.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Fannal C3004 MIPI-DSI
+
+maintainers:
+  - Paulo Pavacic <pavacic.p@gmail.com>
+
+description: |
+  Fannal C3004 is a 480x800 panel which requires DSI DCS
+  initialization sequences.
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: fannal,c3004
+
+  reg: true
+
+  reset-gpios: true
+
+  vdd-supply:
+    description: power supply voltage
+
+  vddio-supply:
+    description: power supply voltage for IO
+
+  width-mm:
+    description: physical panel width [mm]
+
+  height-mm:
+    description: physical panel height [mm]
+
+  panel-timing: true
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "fannal,c3004";
+            reg = <0>;
+            pinctrl-0 = <&pinctrl_mipi_dsi_rst>;
+            pinctrl-names = "default";
+            reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&reg1>;
+            vddio-supply = <&reg2>;
+            width-mm = <93>;
+            height-mm = <56>;
+            panel-timing {
+                clock-frequency = <27000000>;
+                hactive = <480>;
+                vactive = <800>;
+                hfront-porch = <30>;
+                hback-porch = <30>;
+                hsync-len = <8>;
+                vback-porch = <30>;
+                vfront-porch = <30>;
+                vsync-len = <8>;
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 5c22c828ab46..4f4492d0f6c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6427,6 +6427,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/panel/ebbg,ft8719.yaml
 F:	drivers/gpu/drm/panel/panel-ebbg-ft8719.c
 
+DRM DRIVER FOR FANNAL C3004373132019A
+M:	Paulo Pavacic <pavacic.p@gmail.com>
+S:	Maintained
+C:	matrix:r/linux-drm:matrix.org
+F:	Documentation/devicetree/bindings/display/panel/panel-fannal,c3004.yaml
+
 DRM DRIVER FOR FARADAY TVE200 TV ENCODER
 M:	Linus Walleij <linus.walleij@linaro.org>
 S:	Maintained
-- 
2.40.1

[PATCH v3 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel

[Paulo Pavacic]

Fannal C3004 is a 480x800 display made by fannal that requires
DCS initialization sequences.

Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
---
v4 changelog:
 - formatting and style changes
 - change community room
v3 changelog:
 - using generic mipi_dsi_dcs_write_seq
 - removed success prints
 - removed some comments
 - simplified code/removed support for different panels
 - changed namespace from fann to fannal
v2 changelog:
 - renamed from panel-mipi-dsi-bringup
 - only one MAINTAINER e-mail
---
 MAINTAINERS                                |   1 +
 drivers/gpu/drm/panel/Kconfig              |  11 +
 drivers/gpu/drm/panel/Makefile             |   1 +
 drivers/gpu/drm/panel/panel-fannal-c3004.c | 318 +++++++++++++++++++++
 4 files changed, 331 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-fannal-c3004.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f4492d0f6c9..c3187927b763 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6432,6 +6432,7 @@ M:	Paulo Pavacic <pavacic.p@gmail.com>
 S:	Maintained
 C:	matrix:r/linux-drm:matrix.org
 F:	Documentation/devicetree/bindings/display/panel/panel-fannal,c3004.yaml
+F:	drivers/gpu/drm/panel/panel-fannal-c3004.c
 
 DRM DRIVER FOR FARADAY TVE200 TV ENCODER
 M:	Linus Walleij <linus.walleij@linaro.org>
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 2b9d6db7860b..a1041c1e6bf6 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -144,6 +144,17 @@ config DRM_PANEL_ELIDA_KD35T133
 	  KD35T133 controller for 320x480 LCD panels with MIPI-DSI
 	  system interfaces.
 
+config DRM_PANEL_FANNAL_C3004
+	tristate "Fannal C3004 panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	help
+	  Say Y here if you want to enable support for the Fannal C3004
+	  2-lane 480x800 MIPI DSI panel which requires initialization
+	  sequence.
+
+	  If M is selected the module will be called panel-fannal-c3004.
+
 config DRM_PANEL_FEIXIN_K101_IM2BA02
 	tristate "Feixin K101 IM2BA02 panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index ff169781e82d..13c0f00038b5 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_EDP) += panel-edp.o
 obj-$(CONFIG_DRM_PANEL_EBBG_FT8719) += panel-ebbg-ft8719.o
 obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o
+obj-$(CONFIG_DRM_PANEL_FANNAL_C3004) += panel-fannal-c3004.o
 obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o
 obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o
 obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
diff --git a/drivers/gpu/drm/panel/panel-fannal-c3004.c b/drivers/gpu/drm/panel/panel-fannal-c3004.c
new file mode 100644
index 000000000000..c26b69315334
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-fannal-c3004.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MIPI DSI driver for fannal C3004.
+ * Copyright (C) 2023, Zenitel
+ * Author: Paulo Pavacic <pavacic.p@gmail.com>
+ */
+
+// ↓ include headers, static values, static functions ↓
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/media-bus-format.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+
+struct fannal_panel_data {
+	struct drm_panel panel;
+	struct gpio_desc *reset;
+};
+
+static struct fannal_panel_data *
+get_fannal_panel_data_from_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct fannal_panel_data, panel);
+}
+
+static const u32 fannal_bus_formats[] = {
+	MEDIA_BUS_FMT_RGB888_1X24,
+};
+
+// resolution 480p x 800p, 56mmx93mm
+static const struct drm_display_mode fannal_c3004_display_mode = {
+	.clock = 27000,
+	.hdisplay = 480, // display height pixels
+	.hsync_start = 480 + 30, // hdisplay + HBP
+	.hsync_end = 480 + 30 + 8, // hdisplay + HBP + HSync
+	.htotal = 480 + 30 + 8 + 30, // hdisplay + HBP + HSync + HFP
+	.vdisplay = 800, // display width pixels
+	.vsync_start = 800 + 20, // vdisplay + VBP
+	.vsync_end = 800 + 20 + 8, // vdisplay + VBP + VSync
+	.vtotal = 800 + 20 + 8 + 20, // vdisplay + VBP + VSync + VFP
+	.width_mm = 93,
+	.height_mm = 56,
+	.flags = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+};
+
+static void fannal_panel_remove(struct mipi_dsi_device *dsi)
+{
+	struct fannal_panel_data *panel_data = mipi_dsi_get_drvdata(dsi);
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret)
+		dev_err(dev, "error: disable: mipi detach (%d)\n", ret);
+
+	drm_panel_remove(&panel_data->panel);
+}
+
+static int fannal_panel_disable(struct drm_panel *panel)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(panel->dev);
+	struct device *dev = panel->dev;
+	int ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		dev_err(dev, "error: disable: turn display OFF (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(5000, 10000);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "error: disable: enter sleep mode (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int fannal_panel_unprepare(struct drm_panel *panel)
+{
+	struct fannal_panel_data *panel_data =
+		get_fannal_panel_data_from_panel(panel);
+
+	if (panel_data->reset) {
+		gpiod_set_value_cansleep(panel_data->reset, 1);
+		usleep_range(15000, 17000);
+		gpiod_set_value_cansleep(panel_data->reset, 0);
+	}
+
+	return 0;
+}
+
+static void fannal_panel_shutdown(struct mipi_dsi_device *dsi)
+{
+	struct fannal_panel_data *panel_data = mipi_dsi_get_drvdata(dsi);
+
+	fannal_panel_disable(&panel_data->panel);
+	fannal_panel_unprepare(&panel_data->panel);
+}
+
+static int fannal_panel_get_modes(struct drm_panel *panel,
+				  struct drm_connector *connector)
+{
+	struct drm_display_mode *mode;
+	const struct drm_display_mode *panel_display_mode =
+		&fannal_c3004_display_mode;
+
+	mode = drm_mode_duplicate(connector->dev, &fannal_c3004_display_mode);
+	if (!mode) {
+		dev_err(panel->dev, "error: get_modes: add drm mode %ux%u@%u\n",
+			panel_display_mode->hdisplay,
+			panel_display_mode->vdisplay,
+			drm_mode_vrefresh(panel_display_mode));
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	connector->display_info.bus_flags = DRM_BUS_FLAG_DE_LOW |
+					    DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE;
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 fannal_bus_formats,
+					 ARRAY_SIZE(fannal_bus_formats));
+	return 1;
+}
+
+static const struct of_device_id fannal_of_match[] = {
+	{ .compatible = "fannal,c3004" },
+	{ /* sentinel */ }
+};
+
+static int fannal_panel_enable(struct drm_panel *panel)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(panel->dev);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x13);
+	mipi_dsi_generic_write_seq(dsi, 0xEF, 0x08);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x10);
+	mipi_dsi_generic_write_seq(dsi, 0xC0, 0x63, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xC1, 0x0A, 0x0C);
+	mipi_dsi_generic_write_seq(dsi, 0xC2, 0x31, 0x08);
+	mipi_dsi_generic_write_seq(dsi, 0xCC, 0x18);
+
+	mipi_dsi_generic_write_seq(dsi, 0xB0, 0x00, 0x08, 0x10, 0x0E, 0x11,
+				   0x07, 0x08, 0x08, 0x08, 0x25, 0x04, 0x12,
+				   0x0F, 0x2C, 0x30, 0x1F);
+	mipi_dsi_generic_write_seq(dsi, 0xB1, 0x00, 0x11, 0x18, 0x0C, 0x10,
+				   0x05, 0x07, 0x09, 0x08, 0x24, 0x04, 0x11,
+				   0x10, 0x2B, 0x30, 0x1F);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x11);
+	mipi_dsi_generic_write_seq(dsi, 0xB0, 0x4D);
+	mipi_dsi_generic_write_seq(dsi, 0xB1, 0x39);
+	mipi_dsi_generic_write_seq(dsi, 0xB2, 0x87);
+	mipi_dsi_generic_write_seq(dsi, 0xB3, 0x80);
+	mipi_dsi_generic_write_seq(dsi, 0xB5, 0x47);
+	mipi_dsi_generic_write_seq(dsi, 0xB7, 0x8A);
+	mipi_dsi_generic_write_seq(dsi, 0xB8, 0x20);
+	mipi_dsi_generic_write_seq(dsi, 0xB9, 0x10, 0x13);
+	mipi_dsi_generic_write_seq(dsi, 0xC1, 0x78);
+	mipi_dsi_generic_write_seq(dsi, 0xC2, 0x78);
+	mipi_dsi_generic_write_seq(dsi, 0xD0, 0x88);
+
+	//PANEL
+	mipi_dsi_generic_write_seq(dsi, 0xE0, 0x00, 0x00, 0x02);
+	mipi_dsi_generic_write_seq(dsi, 0xE1, 0x04, 0x00, 0x00, 0x00, 0x05,
+				   0x00, 0x00, 0x00, 0x00, 0x20, 0x20);
+	mipi_dsi_generic_write_seq(dsi, 0xE2, 0x00, 0x00, 0x00, 0x00, 0x00,
+				   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				   0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xE3, 0x00, 0x00, 0x33, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xE4, 0x22, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xE5, 0x04, 0x34, 0xAA, 0xAA, 0x06,
+				   0x34, 0xAA, 0xAA, 0x00, 0x00, 0x00, 0x00,
+				   0x00, 0x00, 0x00, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xE6, 0x00, 0x00, 0x33, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xE7, 0x22, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xE8, 0x05, 0x34, 0xAA, 0xAA, 0x07,
+				   0x34, 0xAA, 0xAA, 0x00, 0x00, 0x00, 0x00,
+				   0x00, 0x00, 0x00, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xEB, 0x02, 0x00, 0x40, 0x40, 0x00,
+				   0x00, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xEC, 0x00, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0xED, 0xFA, 0x45, 0x0B, 0xFF, 0xFF,
+				   0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+				   0xFF, 0xB0, 0x54, 0xAF);
+	mipi_dsi_generic_write_seq(dsi, 0xEF, 0x10, 0x0D, 0x04, 0x08, 0x3F,
+				   0x1F);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x13);
+	mipi_dsi_generic_write_seq(dsi, 0xE8, 0x00, 0x0E);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0x11); //MIPI_DCS_EXIT_SLEEP_MODE
+
+	msleep(600);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x13);
+	mipi_dsi_generic_write_seq(dsi, 0xE8, 0x00, 0x0C);
+	msleep(50);
+	mipi_dsi_generic_write_seq(dsi, 0xE8, 0x00, 0x00);
+
+	mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x00);
+	mipi_dsi_generic_write_seq(dsi, 0x29); //MIPI_DCS_SET_DISPLAY_ON
+	msleep(100);
+	return 0;
+}
+
+static int fannal_panel_prepare(struct drm_panel *panel)
+{
+	struct fannal_panel_data *panel_data =
+		get_fannal_panel_data_from_panel(panel);
+
+	/* At lest 10ms needed between power-on and reset-out as RM specifies */
+	usleep_range(10000, 12000);
+
+	if (panel_data->reset) {
+		gpiod_set_value_cansleep(panel_data->reset, 0);
+		/*
+		 * 50ms delay after reset-out, as per manufacturer initalization
+		 * sequence.
+		 */
+		msleep(50);
+	}
+
+	return 0;
+}
+
+static const struct drm_panel_funcs fannal_panel_funcs = {
+	.prepare = fannal_panel_prepare,
+	.unprepare = fannal_panel_unprepare,
+	.enable = fannal_panel_enable,
+	.disable = fannal_panel_disable,
+	.get_modes = fannal_panel_get_modes,
+};
+
+static int fannal_panel_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct fannal_panel_data *panel_data;
+	int ret;
+
+	panel_data = devm_kzalloc(&dsi->dev, sizeof(*panel_data), GFP_KERNEL);
+
+	if (!panel_data)
+		return -ENOMEM;
+
+	panel_data->reset =
+		devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+
+	if (IS_ERR(panel_data->reset)) {
+		return dev_err_probe(
+			dev, PTR_ERR(panel_data->reset),
+			"error: probe: get reset GPIO: (%d) Check the fdt\n",
+			ret);
+	}
+
+	mipi_dsi_set_drvdata(dsi, panel_data);
+
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |
+			  MIPI_DSI_MODE_VSYNC_FLUSH | MIPI_DSI_MODE_VIDEO_HSE |
+			  MIPI_DSI_MODE_NO_EOT_PACKET |
+			  MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_VIDEO;
+	dsi->lanes = 2;
+
+	gpiod_set_value_cansleep(panel_data->reset, 1);
+
+	drm_panel_init(&panel_data->panel, dev, &fannal_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+	dev_set_drvdata(dev, panel_data);
+
+	drm_panel_add(&panel_data->panel);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret) {
+		drm_panel_remove(&panel_data->panel);
+		dev_err(dev, "error: probe fail: can't attach mipi_dsi\n");
+	}
+
+	return ret;
+}
+
+static struct mipi_dsi_driver fannal_panel_driver = {
+	.driver = {
+		.name = "panel-fannal-c3004",
+		.of_match_table = fannal_of_match,
+	},
+	.probe = fannal_panel_probe,
+	.remove = fannal_panel_remove,
+	.shutdown = fannal_panel_shutdown,
+};
+
+module_mipi_dsi_driver(fannal_panel_driver);
+
+MODULE_AUTHOR("Paulo Pavacic <pavacic.p@gmail.com>");
+MODULE_DESCRIPTION("fannal C3004 panel driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, fannal_of_match);
-- 
2.40.1

Re: [PATCH v3 1/3] dt-bindings: add fannal vendor prefix

[Krzysztof Kozlowski]

On 06/06/2023 16:07, Paulo Pavacic wrote:
> Fannal is a one-stop-solution provider for touch displays in industrial
> application, committed to delivering a variety of professional high-quality
> products and technical services globally.
> 
> Website: www.fannal.com
> 
> Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions. However, there's no need to repost patches *only* to add the
tags. The upstream maintainer will do that for acks received on the
version they apply.

https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof

Re: [PATCH v3 2/3] dt-bindings: display: panel: add fannal,c3004

[Krzysztof Kozlowski]

On 06/06/2023 16:07, Paulo Pavacic wrote:
> Added fannal to vendor-prefixes and dt bindings for Fannal C3004.
> Fannal C3004 is a 480x800 MIPI DSI Panel which requires
> DCS initialization sequences with certain delays between certain
> commands.
> 
> Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
> ---
> v4 changelog:
>   - add spaces between properties

???

I pointed out last incorrect versioning. This is v3, not v4. Or is it v4?

What about my tag?

What about my comment?

Best regards,
Krzysztof

Re: [PATCH v3 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel

[Krzysztof Kozlowski]

On 06/06/2023 16:07, Paulo Pavacic wrote:
> Fannal C3004 is a 480x800 display made by fannal that requires
> DCS initialization sequences.
> 
> Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
> ---
> v4 changelog:
>  - formatting and style changes

Are you sure? I see other changes - removal of some code, handling
errors and GPIO.

>  - change community room

What does it mean? Where is this change in this patch?

...

> +
> +static const struct drm_panel_funcs fannal_panel_funcs = {
> +	.prepare = fannal_panel_prepare,
> +	.unprepare = fannal_panel_unprepare,
> +	.enable = fannal_panel_enable,
> +	.disable = fannal_panel_disable,
> +	.get_modes = fannal_panel_get_modes,
> +};
> +
> +static int fannal_panel_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct fannal_panel_data *panel_data;
> +	int ret;
> +
> +	panel_data = devm_kzalloc(&dsi->dev, sizeof(*panel_data), GFP_KERNEL);
> +

Drop blank line.

> +	if (!panel_data)
> +		return -ENOMEM;
> +
> +	panel_data->reset =

You have wrong wrapping here. devm_gpiod_get_optional() goes after =.

> +		devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +

Drop blank line.

> +	if (IS_ERR(panel_data->reset)) {
> +		return dev_err_probe(
> +			dev, PTR_ERR(panel_data->reset),
> +			"error: probe: get reset GPIO: (%d) Check the fdt\n",
> +			ret);
> +	}
> +
> +	mipi_dsi_set_drvdata(dsi, panel_data);
> +
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |
> +			  MIPI_DSI_MODE_VSYNC_FLUSH | MIPI_DSI_MODE_VIDEO_HSE |
> +			  MIPI_DSI_MODE_NO_EOT_PACKET |
> +			  MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_VIDEO;
> +	dsi->lanes = 2;
> +
> +	gpiod_set_value_cansleep(panel_data->reset, 1);
> +
> +	drm_panel_init(&panel_data->panel, dev, &fannal_panel_funcs,
> +		       DRM_MODE_CONNECTOR_DSI);
> +	dev_set_drvdata(dev, panel_data);
> +
> +	drm_panel_add(&panel_data->panel);
> +


Best regards,
Krzysztof

Re: [PATCH v3 2/3] dt-bindings: display: panel: add fannal,c3004

[Paulo Pavacic]

Hello Krzysztof,

uto, 6. lip 2023. u 16:43 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> napisao je:
>
> On 06/06/2023 16:07, Paulo Pavacic wrote:
> > Added fannal to vendor-prefixes and dt bindings for Fannal C3004.
> > Fannal C3004 is a 480x800 MIPI DSI Panel which requires
> > DCS initialization sequences with certain delays between certain
> > commands.
> >
> > Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
> > ---
> > v4 changelog:
> >   - add spaces between properties
>
> ???

Added empty lines between properties in yml file

>
> I pointed out last incorrect versioning. This is v3, not v4. Or is it v4?

It is v4 of the patch but v3 of the patchset. I wasn't sure whether
somebody would complain if I were to name [patch 2/3] in a patch set
with different version. I will try to edit changelog to match patchset
version.

>
> What about my tag?
>

I have changed in MAINTAINERS file from "+C:
matrix:r/mipi-dsi-bringup:matrix.org" to " +C:
matrix:r/linux-drm:matrix.org". So I wasn't sure whether to add it.
I will add it in future version of the patch.

> What about my comment?
>

I thought you wanted me to have more generalized MAINTAINERS community
URI that's why I have changed it to linux-drm. I will remove community
URI in future version of the patch.

> Best regards,
> Krzysztof
>

Thanks,
Paulo

Re: [PATCH v3 2/3] dt-bindings: display: panel: add fannal,c3004

[Krzysztof Kozlowski]

On 07/06/2023 11:29, Paulo Pavacic wrote:
> Hello Krzysztof,
> 
> uto, 6. lip 2023. u 16:43 Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> napisao je:
>>
>> On 06/06/2023 16:07, Paulo Pavacic wrote:
>>> Added fannal to vendor-prefixes and dt bindings for Fannal C3004.
>>> Fannal C3004 is a 480x800 MIPI DSI Panel which requires
>>> DCS initialization sequences with certain delays between certain
>>> commands.
>>>
>>> Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
>>> ---
>>> v4 changelog:
>>>   - add spaces between properties
>>
>> ???
> 
> Added empty lines between properties in yml file
> 
>>
>> I pointed out last incorrect versioning. This is v3, not v4. Or is it v4?
> 
> It is v4 of the patch but v3 of the patchset. I wasn't sure whether
> somebody would complain if I were to name [patch 2/3] in a patch set
> with different version. I will try to edit changelog to match patchset
> version.

Please maintain consistent versioning. There is only one version -
that's the version you are sending.

> 
>>
>> What about my tag?
>>
> 
> I have changed in MAINTAINERS file from "+C:
> matrix:r/mipi-dsi-bringup:matrix.org" to " +C:
> matrix:r/linux-drm:matrix.org". So I wasn't sure whether to add it.
> I will add it in future version of the patch.

Keep the tags you received.

> 
>> What about my comment?
>>
> 
> I thought you wanted me to have more generalized MAINTAINERS community
> URI that's why I have changed it to linux-drm. I will remove community
> URI in future version of the patch.

The chat should be in subsystem entry, not in individual drivers. People
will not hop between 100 IRC rooms to figure out discussion. Just like
we have only one mailing list dedicated to given topic.

Best regards,
Krzysztof

Re: [PATCH v3 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel

[Paulo Pavacic]

Hello,

uto, 6. lip 2023. u 16:45 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> napisao je:
>
> On 06/06/2023 16:07, Paulo Pavacic wrote:
> > Fannal C3004 is a 480x800 display made by fannal that requires
> > DCS initialization sequences.
> >
> > Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
> > ---
> > v4 changelog:
> >  - formatting and style changes
>
> Are you sure? I see other changes - removal of some code, handling
> errors and GPIO.
>
> >  - change community room
>
> What does it mean? Where is this change in this patch?
>
> ...
>
> > +
> > +static const struct drm_panel_funcs fannal_panel_funcs = {
> > +     .prepare = fannal_panel_prepare,
> > +     .unprepare = fannal_panel_unprepare,
> > +     .enable = fannal_panel_enable,
> > +     .disable = fannal_panel_disable,
> > +     .get_modes = fannal_panel_get_modes,
> > +};
> > +
> > +static int fannal_panel_probe(struct mipi_dsi_device *dsi)
> > +{
> > +     struct device *dev = &dsi->dev;
> > +     struct fannal_panel_data *panel_data;
> > +     int ret;
> > +
> > +     panel_data = devm_kzalloc(&dsi->dev, sizeof(*panel_data), GFP_KERNEL);
> > +
>
> Drop blank line.
>
> > +     if (!panel_data)
> > +             return -ENOMEM;
> > +
> > +     panel_data->reset =
>
> You have wrong wrapping here. devm_gpiod_get_optional() goes after =.

I'm not sure why, but clang-format makes it that way. I'm using this
style: https://raw.githubusercontent.com/torvalds/linux/master/.clang-format
Do you have some other style?

>
> > +             devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > +
>
> Drop blank line.
>
> > +     if (IS_ERR(panel_data->reset)) {
> > +             return dev_err_probe(
> > +                     dev, PTR_ERR(panel_data->reset),
> > +                     "error: probe: get reset GPIO: (%d) Check the fdt\n",
> > +                     ret);
> > +     }
> > +
> > +     mipi_dsi_set_drvdata(dsi, panel_data);
> > +
> > +     dsi->format = MIPI_DSI_FMT_RGB888;
> > +     dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |
> > +                       MIPI_DSI_MODE_VSYNC_FLUSH | MIPI_DSI_MODE_VIDEO_HSE |
> > +                       MIPI_DSI_MODE_NO_EOT_PACKET |
> > +                       MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_VIDEO;
> > +     dsi->lanes = 2;
> > +
> > +     gpiod_set_value_cansleep(panel_data->reset, 1);
> > +
> > +     drm_panel_init(&panel_data->panel, dev, &fannal_panel_funcs,
> > +                    DRM_MODE_CONNECTOR_DSI);
> > +     dev_set_drvdata(dev, panel_data);
> > +
> > +     drm_panel_add(&panel_data->panel);
> > +
>
>
> Best regards,
> Krzysztof
>

Thanks,
Paulo

Re: [PATCH v3 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel

[Krzysztof Kozlowski]

On 07/06/2023 15:38, Paulo Pavacic wrote:
> Hello,
> 
> uto, 6. lip 2023. u 16:45 Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> napisao je:
>>
>> On 06/06/2023 16:07, Paulo Pavacic wrote:
>>> Fannal C3004 is a 480x800 display made by fannal that requires
>>> DCS initialization sequences.
>>>
>>> Signed-off-by: Paulo Pavacic <pavacic.p@gmail.com>
>>> ---
>>> v4 changelog:
>>>  - formatting and style changes
>>
>> Are you sure? I see other changes - removal of some code, handling
>> errors and GPIO.
>>
>>>  - change community room
>>
>> What does it mean? Where is this change in this patch?
>>
>> ...
>>
>>> +
>>> +static const struct drm_panel_funcs fannal_panel_funcs = {
>>> +     .prepare = fannal_panel_prepare,
>>> +     .unprepare = fannal_panel_unprepare,
>>> +     .enable = fannal_panel_enable,
>>> +     .disable = fannal_panel_disable,
>>> +     .get_modes = fannal_panel_get_modes,
>>> +};
>>> +
>>> +static int fannal_panel_probe(struct mipi_dsi_device *dsi)
>>> +{
>>> +     struct device *dev = &dsi->dev;
>>> +     struct fannal_panel_data *panel_data;
>>> +     int ret;
>>> +
>>> +     panel_data = devm_kzalloc(&dsi->dev, sizeof(*panel_data), GFP_KERNEL);
>>> +
>>
>> Drop blank line.
>>
>>> +     if (!panel_data)
>>> +             return -ENOMEM;
>>> +
>>> +     panel_data->reset =
>>
>> You have wrong wrapping here. devm_gpiod_get_optional() goes after =.
> 
> I'm not sure why, but clang-format makes it that way. I'm using this
> style: https://raw.githubusercontent.com/torvalds/linux/master/.clang-format
> Do you have some other style?

Linux kernel coding style.

https://elixir.bootlin.com/linux/v6.4-rc5/source/Documentation/process/coding-style.rst

Don't use clang or other non-kernel formatters.


Best regards,
Krzysztof