2023-06-05 15:39:46

by Paulo Pavacic

[permalink] [raw]
Subject: [PATCH v2 0/3] drm/panel: add fannal c3004 panel

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-c3003: 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 | 319 ++++++++++++++++++
6 files changed, 418 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



2023-06-05 15:40:49

by Paulo Pavacic

[permalink] [raw]
Subject: [PATCH v2 2/3] dt-bindings: display: panel: add fannal,c3004

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 <[email protected]>
---
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 <[email protected]>
+
+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..62374c8424b9 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 <[email protected]>
+S: Maintained
+C: matrix:r/mipi-dsi-bringup:matrix.org
+F: Documentation/devicetree/bindings/display/panel/panel-fannal,c3004.yaml
+
DRM DRIVER FOR FARADAY TVE200 TV ENCODER
M: Linus Walleij <[email protected]>
S: Maintained
--
2.40.1


2023-06-05 15:41:31

by Paulo Pavacic

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: add fannal vendor prefix

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: http://www.fannal.com

Signed-off-by: Paulo Pavacic <[email protected]>
---
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


2023-06-05 15:41:43

by Paulo Pavacic

[permalink] [raw]
Subject: [PATCH v2 3/3] drm/panel-fannal-c3003: Add fannal c3004 DSI panel

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

Signed-off-by: Paulo Pavacic <[email protected]>
---
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 | 319 +++++++++++++++++++++
4 files changed, 332 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-fannal-c3004.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 62374c8424b9..93fc1ae6832f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6432,6 +6432,7 @@ M: Paulo Pavacic <[email protected]>
S: Maintained
C: matrix:r/mipi-dsi-bringup: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 <[email protected]>
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..69bf648ad15f
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-fannal-c3004.c
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MIPI DSI driver for fannal C3004.
+ * Copyright (C) 2023, Zenitel
+ * Author: Paulo Pavacic <[email protected]>
+ */
+
+// ↓ 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 | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
+
+ if (IS_ERR(panel_data->reset)) {
+ ret = PTR_ERR(panel_data->reset);
+ dev_err(dev,
+ "error: probe: get reset GPIO: (%d) Check the fdt\n",
+ ret);
+ return 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 <[email protected]>");
+MODULE_DESCRIPTION("fannal C3004 panel driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, fannal_of_match);
--
2.40.1


2023-06-06 07:18:18

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: display: panel: add fannal,c3004

On 05/06/2023 17:35, 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.


> + #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>;
> + };
> + };
> + };


Reviewed-by: Krzysztof Kozlowski <[email protected]>

> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5c22c828ab46..62374c8424b9 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 <[email protected]>
> +S: Maintained
> +C: matrix:r/mipi-dsi-bringup:matrix.org

I don't think we have chat channels for individual, small drivers. Add a
channel for entire subsystem.

Best regards,
Krzysztof


2023-06-06 07:32:22

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] drm/panel-fannal-c3003: Add fannal c3004 DSI panel

On 05/06/2023 17:35, Paulo Pavacic wrote:
> Fannal C3004 is a 480x800 display made by fannal that requires
> DCS initialization sequences.
>
> Signed-off-by: Paulo Pavacic <[email protected]>
> ---

> v3 changelog:
> - using generic mipi_dsi_dcs_write_seq

This is marked as v2?

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


> +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 | GPIOD_FLAGS_BIT_NONEXCLUSIVE);

dev should be in previous line

Why this is nonexclusive? How many instances of same driver are
expecting to get this on one system?

> +
> + if (IS_ERR(panel_data->reset)) {
> + ret = PTR_ERR(panel_data->reset);
> + dev_err(dev,
> + "error: probe: get reset GPIO: (%d) Check the fdt\n",
> + ret);

return dev_err_probe

> + return 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);

So you leave the panel in reset state?
> +
> + 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");

No need to shout, skip exclamation mark.


Best regards,
Krzysztof


2023-06-06 07:41:03

by Paulo Pavacic

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] drm/panel-fannal-c3003: Add fannal c3004 DSI panel

Hello Krzysztof,

uto, 6. lip 2023. u 09:14 Krzysztof Kozlowski
<[email protected]> napisao je:
>
> On 05/06/2023 17:35, Paulo Pavacic wrote:
> > Fannal C3004 is a 480x800 display made by fannal that requires
> > DCS initialization sequences.
> >
> > Signed-off-by: Paulo Pavacic <[email protected]>
> > ---
>
> > v3 changelog:
> > - using generic mipi_dsi_dcs_write_seq
>
> This is marked as v2?

I have ruined numbering. At first this driver was named
panel-mipi-dsi-bringup and parts of it weren't handled under patchset.

>
> > - 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
>
>
> > +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 | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
>
> dev should be in previous line
>
> Why this is nonexclusive? How many instances of same driver are
> expecting to get this on one system?
>
> > +
> > + if (IS_ERR(panel_data->reset)) {
> > + ret = PTR_ERR(panel_data->reset);
> > + dev_err(dev,
> > + "error: probe: get reset GPIO: (%d) Check the fdt\n",
> > + ret);
>
> return dev_err_probe
>
> > + return 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);
>
> So you leave the panel in reset state?
yes

> > +
> > + 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");
>
> No need to shout, skip exclamation mark.
>
>
> Best regards,
> Krzysztof
>

Thanks,
Paulo

2023-06-06 07:41:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: add fannal vendor prefix

On 05/06/2023 17:35, 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: http://www.fannal.com
>
> Signed-off-by: Paulo Pavacic <[email protected]>


Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-06-06 10:01:09

by Paulo Pavacic

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: display: panel: add fannal,c3004

Hello Krzysztof,

uto, 6. lip 2023. u 09:10 Krzysztof Kozlowski
<[email protected]> napisao je:
>
> On 05/06/2023 17:35, 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.
>
>
> > + #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>;
> > + };
> > + };
> > + };
>
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5c22c828ab46..62374c8424b9 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 <[email protected]>
> > +S: Maintained
> > +C: matrix:r/mipi-dsi-bringup:matrix.org
>
> I don't think we have chat channels for individual, small drivers. Add a
> channel for entire subsystem.
Okay I will add matrix:r/linux-drm:matrix.org
>
> Best regards,
> Krzysztof
>

Best regards,
Paulo

2023-06-06 15:14:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: display: panel: add fannal,c3004

On 06/06/2023 11:53, Paulo Pavacic wrote:
>>> + #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>;
>>> + };
>>> + };
>>> + };
>>
>>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>

So this you decided to ignore?

>>
>>> +...
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 5c22c828ab46..62374c8424b9 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 <[email protected]>
>>> +S: Maintained
>>> +C: matrix:r/mipi-dsi-bringup:matrix.org
>>
>> I don't think we have chat channels for individual, small drivers. Add a
>> channel for entire subsystem.
> Okay I will add matrix:r/linux-drm:matrix.org

I meant, we do not have entries for individual drivers in MAINTAINERS
with mailing lists or with chats. The entries covering entire subsystems
have such links. We are not going to sign up for 100 different channels,
each one per driver.

Best regards,
Krzysztof


2023-06-06 15:32:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] drm/panel-fannal-c3003: Add fannal c3004 DSI panel

On 06/06/2023 09:36, Paulo Pavacic wrote:
>>> +
>>> + if (IS_ERR(panel_data->reset)) {
>>> + ret = PTR_ERR(panel_data->reset);
>>> + dev_err(dev,
>>> + "error: probe: get reset GPIO: (%d) Check the fdt\n",
>>> + ret);
>>
>> return dev_err_probe
>>
>>> + return 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);
>>
>> So you leave the panel in reset state?
> yes

Then why do you initialize GPIO to bring it out of reset and them
immediately put into reset back? Just keep it in reset.

Best regards,
Krzysztof