* [PATCH v4 0/3] drm/panel: add fannal c3004 panel
@ 2023-06-07 15:11 Paulo Pavacic
2023-06-07 15:11 ` [PATCH v4 1/3] dt-bindings: add fannal vendor prefix Paulo Pavacic
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Paulo Pavacic @ 2023-06-07 15:11 UTC (permalink / raw)
To: neil.armstrong, sam, airlied, daniel, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: devicetree, linux-kernel, dri-devel, 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 | 6 +
drivers/gpu/drm/panel/Kconfig | 11 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-fannal-c3004.c | 314 ++++++++++++++++++
6 files changed, 412 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
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v4 1/3] dt-bindings: add fannal vendor prefix 2023-06-07 15:11 [PATCH v4 0/3] drm/panel: add fannal c3004 panel Paulo Pavacic @ 2023-06-07 15:11 ` Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 2/3] dt-bindings: display: panel: add fannal,c3004 Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel Paulo Pavacic 2 siblings, 0 replies; 22+ messages in thread From: Paulo Pavacic @ 2023-06-07 15:11 UTC (permalink / raw) To: neil.armstrong, sam, airlied, daniel, robh+dt, krzysztof.kozlowski+dt, conor+dt Cc: devicetree, Krzysztof Kozlowski, linux-kernel, dri-devel, 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> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/3] dt-bindings: display: panel: add fannal,c3004 2023-06-07 15:11 [PATCH v4 0/3] drm/panel: add fannal c3004 panel Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 1/3] dt-bindings: add fannal vendor prefix Paulo Pavacic @ 2023-06-07 15:11 ` Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel Paulo Pavacic 2 siblings, 0 replies; 22+ messages in thread From: Paulo Pavacic @ 2023-06-07 15:11 UTC (permalink / raw) To: neil.armstrong, sam, airlied, daniel, robh+dt, krzysztof.kozlowski+dt, conor+dt Cc: devicetree, Krzysztof Kozlowski, linux-kernel, dri-devel, 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> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- v4 changelog: - removed driver's community from MAINTAINERS file v2 changelog: - added empty lines between properties in yml - renamed yml file - refactored yml file to describe fannal,c3004 - added matrix URI to MAINTAINERS v1 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 | 5 ++ 2 files changed, 83 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 = <®1>; + vddio-supply = <®2>; + 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..978133f87c5e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6427,6 +6427,11 @@ 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 +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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-07 15:11 [PATCH v4 0/3] drm/panel: add fannal c3004 panel Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 1/3] dt-bindings: add fannal vendor prefix Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 2/3] dt-bindings: display: panel: add fannal,c3004 Paulo Pavacic @ 2023-06-07 15:11 ` Paulo Pavacic 2023-06-15 19:54 ` Linus Walleij 2 siblings, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-06-07 15:11 UTC (permalink / raw) To: neil.armstrong, sam, airlied, daniel, robh+dt, krzysztof.kozlowski+dt, conor+dt Cc: devicetree, linux-kernel, dri-devel, 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: - removal of blank lines, code wrapping changes - reset pin initialization handling changes v3 changelog: - formatting and style changes - remove BIT_NONEXCLUSIVE flag from reset pin - use dev_err_probe instead of dev_err v2 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 v1 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 | 314 +++++++++++++++++++++ 4 files changed, 327 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-fannal-c3004.c diff --git a/MAINTAINERS b/MAINTAINERS index 978133f87c5e..7ad37ecd7e09 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6431,6 +6431,7 @@ DRM DRIVER FOR FANNAL C3004373132019A M: Paulo Pavacic <pavacic.p@gmail.com> S: Maintained 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..002c424f2486 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-fannal-c3004.c @@ -0,0 +1,314 @@ +// 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_HIGH); + 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; + + 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-07 15:11 ` [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel Paulo Pavacic @ 2023-06-15 19:54 ` Linus Walleij 2023-06-16 9:57 ` Paulo Pavacic 0 siblings, 1 reply; 22+ messages in thread From: Linus Walleij @ 2023-06-15 19:54 UTC (permalink / raw) To: Paulo Pavacic Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt Hi Paulo, thanks for your patch! Overall this looks very good. I doubt that the display controller is actually by Fannal, but I guess you tried to find out? We usually try to identify the underlying display controller so the driver can be named after it and reused for more display panels. Some minor questions/nitpicks below. On Wed, Jun 7, 2023 at 5:11 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > +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); Why is everything using mipi_dsi_generic_write_seq() instead of mipi_dsi_dcs_write_seq()? Especially here, because 0x11 is certainly a command: > + mipi_dsi_generic_write_seq(dsi, 0x11); //MIPI_DCS_EXIT_SLEEP_MODE Instead use: ret = mipi_dsi_dcs_exit_sleep_mode(dsi); if (ret) return ret; > + mipi_dsi_generic_write_seq(dsi, 0x29); //MIPI_DCS_SET_DISPLAY_ON Instead use: ret = mipi_dsi_dcs_set_display_on(dsi); if (ret) return ret; Yours, Linus Walleij ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-15 19:54 ` Linus Walleij @ 2023-06-16 9:57 ` Paulo Pavacic 2023-06-16 11:43 ` Linus Walleij 0 siblings, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-06-16 9:57 UTC (permalink / raw) To: Linus Walleij Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt Hello Linus, thank you for the comments. čet, 15. lip 2023. u 21:55 Linus Walleij <linus.walleij@linaro.org> napisao je: > > Hi Paulo, > > thanks for your patch! > > Overall this looks very good. > > I doubt that the display controller is actually by Fannal, but I guess > you tried to find out? We usually try to identify the underlying display > controller so the driver can be named after it and reused for more > display panels. Yes, of course, the controller is ST7701S. > > Some minor questions/nitpicks below. > > On Wed, Jun 7, 2023 at 5:11 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > > +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); > > Why is everything using mipi_dsi_generic_write_seq() instead of > mipi_dsi_dcs_write_seq()? Okay, I will replace it. > > Especially here, because 0x11 is certainly a command: > > > + mipi_dsi_generic_write_seq(dsi, 0x11); //MIPI_DCS_EXIT_SLEEP_MODE > > Instead use: > > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > if (ret) > return ret; > > > > + mipi_dsi_generic_write_seq(dsi, 0x29); //MIPI_DCS_SET_DISPLAY_ON > > Instead use: > > ret = mipi_dsi_dcs_set_display_on(dsi); > if (ret) > return ret; > Okay I will replace all commands with a functions found here: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_mipi_dsi.c#L995 > Yours, > Linus Walleij Best regards, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-16 9:57 ` Paulo Pavacic @ 2023-06-16 11:43 ` Linus Walleij 2023-06-16 12:31 ` Paulo Pavacic 0 siblings, 1 reply; 22+ messages in thread From: Linus Walleij @ 2023-06-16 11:43 UTC (permalink / raw) To: Paulo Pavacic Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt On Fri, Jun 16, 2023 at 11:57 AM Paulo Pavacic <pavacic.p@gmail.com> wrote: > čet, 15. lip 2023. u 21:55 Linus Walleij <linus.walleij@linaro.org> napisao je: > > > > I doubt that the display controller is actually by Fannal, but I guess > > you tried to find out? We usually try to identify the underlying display > > controller so the driver can be named after it and reused for more > > display panels. > > Yes, of course, the controller is ST7701S. Hm did you try to just refactor drivers/gpu/drm/panel/panel-sitronix-st7701.c to support your new panel? One major reason would be that that driver knows what commands actually mean and have #defines for them. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-16 11:43 ` Linus Walleij @ 2023-06-16 12:31 ` Paulo Pavacic 2023-06-16 12:52 ` Linus Walleij 0 siblings, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-06-16 12:31 UTC (permalink / raw) To: Linus Walleij Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt pet, 16. lip 2023. u 13:44 Linus Walleij <linus.walleij@linaro.org> napisao je: > > On Fri, Jun 16, 2023 at 11:57 AM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > čet, 15. lip 2023. u 21:55 Linus Walleij <linus.walleij@linaro.org> napisao je: > > > > > > I doubt that the display controller is actually by Fannal, but I guess > > > you tried to find out? We usually try to identify the underlying display > > > controller so the driver can be named after it and reused for more > > > display panels. > > > > Yes, of course, the controller is ST7701S. > > Hm did you try to just refactor > drivers/gpu/drm/panel/panel-sitronix-st7701.c > to support your new panel? > Yes I have tried, but there are too many changes needed and I wasn't sure whether I would be breaking compatibility with st7701 based panels. > One major reason would be that that driver knows what > commands actually mean and have #defines for them. > > Yours, > Linus Walleij Best regards, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-16 12:31 ` Paulo Pavacic @ 2023-06-16 12:52 ` Linus Walleij 2023-06-21 15:08 ` Paulo Pavacic 0 siblings, 1 reply; 22+ messages in thread From: Linus Walleij @ 2023-06-16 12:52 UTC (permalink / raw) To: Paulo Pavacic Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt On Fri, Jun 16, 2023 at 2:31 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > pet, 16. lip 2023. u 13:44 Linus Walleij <linus.walleij@linaro.org> napisao je: > > > > On Fri, Jun 16, 2023 at 11:57 AM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > > čet, 15. lip 2023. u 21:55 Linus Walleij <linus.walleij@linaro.org> napisao je: > > > > > > > > I doubt that the display controller is actually by Fannal, but I guess > > > > you tried to find out? We usually try to identify the underlying display > > > > controller so the driver can be named after it and reused for more > > > > display panels. > > > > > > Yes, of course, the controller is ST7701S. > > > > Hm did you try to just refactor > > drivers/gpu/drm/panel/panel-sitronix-st7701.c > > to support your new panel? > > Yes I have tried, but there are too many changes needed and I wasn't > sure whether I would be breaking compatibility with st7701 based > panels. For the community it creates a problem that now two drivers for similar hardware need to be maintained, and that burden will land on the DRM maintainers. For this reason it would be better if a joint driver could be created. I am sure the users of the old driver will be willing to test patches to make sure their devices keep working. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-16 12:52 ` Linus Walleij @ 2023-06-21 15:08 ` Paulo Pavacic 2023-06-22 8:22 ` Linus Walleij 0 siblings, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-06-21 15:08 UTC (permalink / raw) To: Linus Walleij Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt pet, 16. lip 2023. u 14:53 Linus Walleij <linus.walleij@linaro.org> napisao je: > > On Fri, Jun 16, 2023 at 2:31 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > pet, 16. lip 2023. u 13:44 Linus Walleij <linus.walleij@linaro.org> napisao je: > > > > > > On Fri, Jun 16, 2023 at 11:57 AM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > > > čet, 15. lip 2023. u 21:55 Linus Walleij <linus.walleij@linaro.org> napisao je: > > > > > > > > > > I doubt that the display controller is actually by Fannal, but I guess > > > > > you tried to find out? We usually try to identify the underlying display > > > > > controller so the driver can be named after it and reused for more > > > > > display panels. > > > > > > > > Yes, of course, the controller is ST7701S. > > > > > > Hm did you try to just refactor > > > drivers/gpu/drm/panel/panel-sitronix-st7701.c > > > to support your new panel? > > > > Yes I have tried, but there are too many changes needed and I wasn't > > sure whether I would be breaking compatibility with st7701 based > > panels. > > For the community it creates a problem that now two drivers for similar > hardware need to be maintained, and that burden will land on the DRM > maintainers. For this reason it would be better if a joint driver could > be created. I will try modifying st7701, but that seems like a big task since currently st7701 crashes kernel (5.15) for me and I have seen suggestions to use raydium driver over st7701. Also I guess I should first read some more documentation and compare st7701 to st7701s. I currently can't reserve a lot of time for that. > > I am sure the users of the old driver will be willing to test patches to > make sure their devices keep working. A lot of modifications to st7701 are required. I believe it would result in a driver that doesn't look or work the same. e.g compare delays between initialization sequences of panel-fannal-c3004 and panel-st7701. I think it would be optimal to create st7701s driver and have special handling for st7701s panels. If there was a flag for whether panel is st7701 or st7701s it would end up looking like a mess. > > Yours, > Linus Walleij Thank you for your time, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-21 15:08 ` Paulo Pavacic @ 2023-06-22 8:22 ` Linus Walleij 2023-07-06 15:18 ` Paulo Pavacic 2023-07-08 8:22 ` Jagan Teki 0 siblings, 2 replies; 22+ messages in thread From: Linus Walleij @ 2023-06-22 8:22 UTC (permalink / raw) To: Paulo Pavacic, Jagan Teki, Marek Vasut, Maya Matuszczyk Cc: neil.armstrong, conor+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, krzysztof.kozlowski+dt On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > A lot of modifications to st7701 are required. I believe it would > result in a driver that doesn't look or work the same. e.g compare > delays between initialization sequences of panel-fannal-c3004 and > panel-st7701. I think it would be optimal to create st7701s driver and > have special handling for st7701s panels. If there was a flag for > whether panel is st7701 or st7701s it would end up looking like a > mess. What matters is if the original authors of the old st7701 driver are around and reviewing and testing patches at all. What we need is active maintainers. (Added Jagan, Marek & Maya). I buy the reasoning that the st7701s is perhaps substantially different from st7701. If st7701s is very different then I suppose it needs a separate driver, then all we need to to name the driver properly, i.e. panel-sitronix-st7701s.c. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-22 8:22 ` Linus Walleij @ 2023-07-06 15:18 ` Paulo Pavacic 2023-07-06 15:26 ` Marek Vasut 2023-07-08 8:22 ` Jagan Teki 1 sibling, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-07-06 15:18 UTC (permalink / raw) To: Linus Walleij Cc: Marek Vasut, neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, Jagan Teki, Maya Matuszczyk Hello Linus, čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: > > On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > > A lot of modifications to st7701 are required. I believe it would > > result in a driver that doesn't look or work the same. e.g compare > > delays between initialization sequences of panel-fannal-c3004 and > > panel-st7701. I think it would be optimal to create st7701s driver and > > have special handling for st7701s panels. If there was a flag for > > whether panel is st7701 or st7701s it would end up looking like a > > mess. > > What matters is if the original authors of the old st7701 driver are > around and reviewing and testing patches at all. What we need is > active maintainers. (Added Jagan, Marek & Maya). > > I buy the reasoning that the st7701s is perhaps substantially different > from st7701. > > If st7701s is very different then I suppose it needs a separate driver, > then all we need to to name the driver properly, i.e. > panel-sitronix-st7701s.c. I had in person talk with Paul Kocialkowski and I have concluded that this is the best solution. I believe I should rename it to st7701s due to the hardware changes. I would like to create V5 patch with driver renamed to st7701s. Please let me know if you agree / disagree. > > Yours, > Linus Walleij Thank you for your time, Paulo Pavačić ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-06 15:18 ` Paulo Pavacic @ 2023-07-06 15:26 ` Marek Vasut 2023-07-07 15:26 ` Paulo Pavacic 2023-07-08 7:11 ` Paulo Pavačić 0 siblings, 2 replies; 22+ messages in thread From: Marek Vasut @ 2023-07-06 15:26 UTC (permalink / raw) To: Paulo Pavacic, Linus Walleij Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, Jagan Teki, Maya Matuszczyk On 7/6/23 17:18, Paulo Pavacic wrote: > Hello Linus, > > čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: >> >> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: >> >>> A lot of modifications to st7701 are required. I believe it would >>> result in a driver that doesn't look or work the same. e.g compare >>> delays between initialization sequences of panel-fannal-c3004 and >>> panel-st7701. I think it would be optimal to create st7701s driver and >>> have special handling for st7701s panels. If there was a flag for >>> whether panel is st7701 or st7701s it would end up looking like a >>> mess. >> >> What matters is if the original authors of the old st7701 driver are >> around and reviewing and testing patches at all. What we need is >> active maintainers. (Added Jagan, Marek & Maya). >> >> I buy the reasoning that the st7701s is perhaps substantially different >> from st7701. >> >> If st7701s is very different then I suppose it needs a separate driver, >> then all we need to to name the driver properly, i.e. >> panel-sitronix-st7701s.c. > > I had in person talk with Paul Kocialkowski and I have concluded that > this is the best solution. > I believe I should rename it to st7701s due to the hardware changes. I > would like to create V5 patch with driver renamed to st7701s. > Please let me know if you agree / disagree. If I recall it right, the ST7701 and ST7701S are basically the same chip, aren't they ? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-06 15:26 ` Marek Vasut @ 2023-07-07 15:26 ` Paulo Pavacic 2023-07-08 11:07 ` Marek Vasut 2023-07-08 7:11 ` Paulo Pavačić 1 sibling, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-07-07 15:26 UTC (permalink / raw) To: Marek Vasut Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk [-- Attachment #1: Type: text/plain, Size: 2396 bytes --] Hello Marek, čet, 6. srp 2023. u 17:26 Marek Vasut <marex@denx.de> napisao je: > > On 7/6/23 17:18, Paulo Pavacic wrote: > > Hello Linus, > > > > čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: > >> > >> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > >> > >>> A lot of modifications to st7701 are required. I believe it would > >>> result in a driver that doesn't look or work the same. e.g compare > >>> delays between initialization sequences of panel-fannal-c3004 and > >>> panel-st7701. I think it would be optimal to create st7701s driver and > >>> have special handling for st7701s panels. If there was a flag for > >>> whether panel is st7701 or st7701s it would end up looking like a > >>> mess. > >> > >> What matters is if the original authors of the old st7701 driver are > >> around and reviewing and testing patches at all. What we need is > >> active maintainers. (Added Jagan, Marek & Maya). > >> > >> I buy the reasoning that the st7701s is perhaps substantially different > >> from st7701. > >> > >> If st7701s is very different then I suppose it needs a separate driver, > >> then all we need to to name the driver properly, i.e. > >> panel-sitronix-st7701s.c. > > > > I had in person talk with Paul Kocialkowski and I have concluded that > > this is the best solution. > > I believe I should rename it to st7701s due to the hardware changes. I > > would like to create V5 patch with driver renamed to st7701s. > > Please let me know if you agree / disagree. > > If I recall it right, the ST7701 and ST7701S are basically the same > chip, aren't they ? I'm currently exploring all the differences. There aren't a lot of differences, but there are some. So far I can see that default register values are different, new previously unused registers are now used and there has been some reordering of how info is placed in registers [1] (data bits are in different order). Moreover, instructions to some commands have been changed and meaning of what data bits mean [2][3]. Also, new features have been added [2]; there is now PCLKS 3 for example. You can see few differences in following images. Same images were attached in this mail: [1] https://ibb.co/NmgbZmy - GAMACTRL_st7701.png [2] https://ibb.co/G79y235 - PCLKS2.png Thank you for your time, Paulo [-- Attachment #2: PCLKS2.png --] [-- Type: image/png, Size: 173528 bytes --] [-- Attachment #3: GAMCTRL_st7701.png --] [-- Type: image/png, Size: 313421 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-07 15:26 ` Paulo Pavacic @ 2023-07-08 11:07 ` Marek Vasut 2023-07-12 12:07 ` Paulo Pavacic 0 siblings, 1 reply; 22+ messages in thread From: Marek Vasut @ 2023-07-08 11:07 UTC (permalink / raw) To: Paulo Pavacic Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk On 7/7/23 17:26, Paulo Pavacic wrote: > Hello Marek, Hi, > čet, 6. srp 2023. u 17:26 Marek Vasut <marex@denx.de> napisao je: >> >> On 7/6/23 17:18, Paulo Pavacic wrote: >>> Hello Linus, >>> >>> čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: >>>> >>>> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: >>>> >>>>> A lot of modifications to st7701 are required. I believe it would >>>>> result in a driver that doesn't look or work the same. e.g compare >>>>> delays between initialization sequences of panel-fannal-c3004 and >>>>> panel-st7701. I think it would be optimal to create st7701s driver and >>>>> have special handling for st7701s panels. If there was a flag for >>>>> whether panel is st7701 or st7701s it would end up looking like a >>>>> mess. >>>> >>>> What matters is if the original authors of the old st7701 driver are >>>> around and reviewing and testing patches at all. What we need is >>>> active maintainers. (Added Jagan, Marek & Maya). >>>> >>>> I buy the reasoning that the st7701s is perhaps substantially different >>>> from st7701. >>>> >>>> If st7701s is very different then I suppose it needs a separate driver, >>>> then all we need to to name the driver properly, i.e. >>>> panel-sitronix-st7701s.c. >>> >>> I had in person talk with Paul Kocialkowski and I have concluded that >>> this is the best solution. >>> I believe I should rename it to st7701s due to the hardware changes. I >>> would like to create V5 patch with driver renamed to st7701s. >>> Please let me know if you agree / disagree. >> >> If I recall it right, the ST7701 and ST7701S are basically the same >> chip, aren't they ? > > I'm currently exploring all the differences. There aren't a lot of > differences, but there are some. > So far I can see that default register values are different, new > previously unused registers are now used and there has been some > reordering of how info is placed in registers [1] (data bits are in > different order). Moreover, instructions to some commands have been > changed and meaning of what data bits mean [2][3]. Also, new features > have been added [2]; there is now PCLKS 3 for example. > > You can see few differences in following images. Same images were > attached in this mail: > [1] https://ibb.co/NmgbZmy - GAMACTRL_st7701.png > [2] https://ibb.co/G79y235 - PCLKS2.png Ouch. I wonder if this is still something that can be abstracted out with some helper accessor functions like: if (model == ST7701) write something else write the other layout Or whether it makes sense to outright have a separate driver. The later would introduce duplication, but maybe that much duplication is OK. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-08 11:07 ` Marek Vasut @ 2023-07-12 12:07 ` Paulo Pavacic 2023-07-12 13:42 ` Marek Vasut 0 siblings, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-07-12 12:07 UTC (permalink / raw) To: Marek Vasut Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk Hello all, sub, 8. srp 2023. u 14:53 Marek Vasut <marex@denx.de> napisao je: > > On 7/7/23 17:26, Paulo Pavacic wrote: > > Hello Marek, > > Hi, > > > čet, 6. srp 2023. u 17:26 Marek Vasut <marex@denx.de> napisao je: > >> > >> On 7/6/23 17:18, Paulo Pavacic wrote: > >>> Hello Linus, > >>> > >>> čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: > >>>> > >>>> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > >>>> > >>>>> A lot of modifications to st7701 are required. I believe it would > >>>>> result in a driver that doesn't look or work the same. e.g compare > >>>>> delays between initialization sequences of panel-fannal-c3004 and > >>>>> panel-st7701. I think it would be optimal to create st7701s driver and > >>>>> have special handling for st7701s panels. If there was a flag for > >>>>> whether panel is st7701 or st7701s it would end up looking like a > >>>>> mess. > >>>> > >>>> What matters is if the original authors of the old st7701 driver are > >>>> around and reviewing and testing patches at all. What we need is > >>>> active maintainers. (Added Jagan, Marek & Maya). > >>>> > >>>> I buy the reasoning that the st7701s is perhaps substantially different > >>>> from st7701. > >>>> > >>>> If st7701s is very different then I suppose it needs a separate driver, > >>>> then all we need to to name the driver properly, i.e. > >>>> panel-sitronix-st7701s.c. > >>> > >>> I had in person talk with Paul Kocialkowski and I have concluded that > >>> this is the best solution. > >>> I believe I should rename it to st7701s due to the hardware changes. I > >>> would like to create V5 patch with driver renamed to st7701s. > >>> Please let me know if you agree / disagree. > >> > >> If I recall it right, the ST7701 and ST7701S are basically the same > >> chip, aren't they ? > > > > I'm currently exploring all the differences. There aren't a lot of > > differences, but there are some. > > So far I can see that default register values are different, new > > previously unused registers are now used and there has been some > > reordering of how info is placed in registers [1] (data bits are in > > different order). Moreover, instructions to some commands have been > > changed and meaning of what data bits mean [2][3]. Also, new features > > have been added [2]; there is now PCLKS 3 for example. > > > > You can see few differences in following images. Same images were > > attached in this mail: > > [1] https://ibb.co/NmgbZmy - GAMACTRL_st7701.png > > [2] https://ibb.co/G79y235 - PCLKS2.png > > Ouch. I wonder if this is still something that can be abstracted out > with some helper accessor functions like: > > if (model == ST7701) > write something > else > write the other layout > > Or whether it makes sense to outright have a separate driver. The later > would introduce duplication, but maybe that much duplication is OK. I would like to create new driver because panel-st7701 seems to be outdated and is using non-standard macro (ST7701_WRITE()) and for me it is crashing kernel 5.15. Does anyone have similar issues with it? Br, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-12 12:07 ` Paulo Pavacic @ 2023-07-12 13:42 ` Marek Vasut 2023-07-12 15:10 ` Paulo Pavacic 0 siblings, 1 reply; 22+ messages in thread From: Marek Vasut @ 2023-07-12 13:42 UTC (permalink / raw) To: Paulo Pavacic Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk On 7/12/23 14:07, Paulo Pavacic wrote: > Hello all, > > sub, 8. srp 2023. u 14:53 Marek Vasut <marex@denx.de> napisao je: >> >> On 7/7/23 17:26, Paulo Pavacic wrote: >>> Hello Marek, >> >> Hi, >> >>> čet, 6. srp 2023. u 17:26 Marek Vasut <marex@denx.de> napisao je: >>>> >>>> On 7/6/23 17:18, Paulo Pavacic wrote: >>>>> Hello Linus, >>>>> >>>>> čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: >>>>>> >>>>>> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: >>>>>> >>>>>>> A lot of modifications to st7701 are required. I believe it would >>>>>>> result in a driver that doesn't look or work the same. e.g compare >>>>>>> delays between initialization sequences of panel-fannal-c3004 and >>>>>>> panel-st7701. I think it would be optimal to create st7701s driver and >>>>>>> have special handling for st7701s panels. If there was a flag for >>>>>>> whether panel is st7701 or st7701s it would end up looking like a >>>>>>> mess. >>>>>> >>>>>> What matters is if the original authors of the old st7701 driver are >>>>>> around and reviewing and testing patches at all. What we need is >>>>>> active maintainers. (Added Jagan, Marek & Maya). >>>>>> >>>>>> I buy the reasoning that the st7701s is perhaps substantially different >>>>>> from st7701. >>>>>> >>>>>> If st7701s is very different then I suppose it needs a separate driver, >>>>>> then all we need to to name the driver properly, i.e. >>>>>> panel-sitronix-st7701s.c. >>>>> >>>>> I had in person talk with Paul Kocialkowski and I have concluded that >>>>> this is the best solution. >>>>> I believe I should rename it to st7701s due to the hardware changes. I >>>>> would like to create V5 patch with driver renamed to st7701s. >>>>> Please let me know if you agree / disagree. >>>> >>>> If I recall it right, the ST7701 and ST7701S are basically the same >>>> chip, aren't they ? >>> >>> I'm currently exploring all the differences. There aren't a lot of >>> differences, but there are some. >>> So far I can see that default register values are different, new >>> previously unused registers are now used and there has been some >>> reordering of how info is placed in registers [1] (data bits are in >>> different order). Moreover, instructions to some commands have been >>> changed and meaning of what data bits mean [2][3]. Also, new features >>> have been added [2]; there is now PCLKS 3 for example. >>> >>> You can see few differences in following images. Same images were >>> attached in this mail: >>> [1] https://ibb.co/NmgbZmy - GAMACTRL_st7701.png >>> [2] https://ibb.co/G79y235 - PCLKS2.png >> >> Ouch. I wonder if this is still something that can be abstracted out >> with some helper accessor functions like: >> >> if (model == ST7701) >> write something >> else >> write the other layout >> >> Or whether it makes sense to outright have a separate driver. The later >> would introduce duplication, but maybe that much duplication is OK. > > I would like to create new driver because panel-st7701 seems to be > outdated and is using non-standard macro (ST7701_WRITE() There is no such macro: $ git grep ST7701_WRITE drivers/gpu/drm/panel/ | wc -l 0 There never was such a macro used in the driver either, are you sure you are not using some hacked up patched downstream fork of the driver ? $ git log -p next/master -- drivers/gpu/drm/panel/panel-sitronix-st7701.c | grep ST7701_WRITE | wc -l 0 > ) and for me > it is crashing kernel 5.15. Have you based all the aforementioned discussion and argumentation on year and half old Linux 5.15.y code base too ? If so, you are missing many patches: $ git log --oneline --no-merges v5.15..next/master -- drivers/gpu/drm/panel/panel-sitronix-st7701.c 5a2854e577dc2 drm: panel: Add orientation support for st7701 e89838968ee44 drm: panel: Add Elida KD50T048A to Sitronix ST7701 driver c62102165dd79 drm/panel/panel-sitronix-st7701: Remove panel on DSI attach failure 49ee766b364ed drm/panel/panel-sitronix-st7701: Clean up CMDnBKx selection c1cdee9b685a1 drm/panel/panel-sitronix-st7701: Fix RTNI calculation 57b2efce45ef5 drm/panel/panel-sitronix-st7701: Add Densitron DMT028VGHMCMI-1A TFT 42542c7904cf2 drm/panel/panel-sitronix-st7701: Split GIP and init sequences 83b7a8e7e88e7 drm/panel/panel-sitronix-st7701: Parametrize voltage and timing de2b4917843cd drm/panel/panel-sitronix-st7701: Infer horizontal pixel count from TFT mode 82f9cee25598a drm/panel/panel-sitronix-st7701: Adjust porch control bitfield name 1ba85119afb5e drm/panel/panel-sitronix-st7701: Infer vertical line count from TFT mode 779c84fea3dbd drm/panel/panel-sitronix-st7701: Make gamma correction TFT specific 7fa8e07128ed6 drm/panel/panel-sitronix-st7701: Make voltage supplies common to ST7701 a6c225be3da7e drm/panel/panel-sitronix-st7701: Enable DSI burst mode, LPM, non-continuous clock 6f481afe220d3 drm/panel/panel-sitronix-st7701: Make DSI mode flags common to ST7701 79abca2b39900 drm/mipi-dsi: Make remove callback return void > Does anyone have similar issues with it? No, I am using it in production. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-12 13:42 ` Marek Vasut @ 2023-07-12 15:10 ` Paulo Pavacic 2023-07-12 17:25 ` Marek Vasut 0 siblings, 1 reply; 22+ messages in thread From: Paulo Pavacic @ 2023-07-12 15:10 UTC (permalink / raw) To: Marek Vasut Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk Hi Marek, sri, 12. srp 2023. u 15:42 Marek Vasut <marex@denx.de> napisao je: > > On 7/12/23 14:07, Paulo Pavacic wrote: > > Hello all, > > > > sub, 8. srp 2023. u 14:53 Marek Vasut <marex@denx.de> napisao je: > >> > >> On 7/7/23 17:26, Paulo Pavacic wrote: > >>> Hello Marek, > >> > >> Hi, > >> > >>> čet, 6. srp 2023. u 17:26 Marek Vasut <marex@denx.de> napisao je: > >>>> > >>>> On 7/6/23 17:18, Paulo Pavacic wrote: > >>>>> Hello Linus, > >>>>> > >>>>> čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: > >>>>>> > >>>>>> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > >>>>>> > >>>>>>> A lot of modifications to st7701 are required. I believe it would > >>>>>>> result in a driver that doesn't look or work the same. e.g compare > >>>>>>> delays between initialization sequences of panel-fannal-c3004 and > >>>>>>> panel-st7701. I think it would be optimal to create st7701s driver and > >>>>>>> have special handling for st7701s panels. If there was a flag for > >>>>>>> whether panel is st7701 or st7701s it would end up looking like a > >>>>>>> mess. > >>>>>> > >>>>>> What matters is if the original authors of the old st7701 driver are > >>>>>> around and reviewing and testing patches at all. What we need is > >>>>>> active maintainers. (Added Jagan, Marek & Maya). > >>>>>> > >>>>>> I buy the reasoning that the st7701s is perhaps substantially different > >>>>>> from st7701. > >>>>>> > >>>>>> If st7701s is very different then I suppose it needs a separate driver, > >>>>>> then all we need to to name the driver properly, i.e. > >>>>>> panel-sitronix-st7701s.c. > >>>>> > >>>>> I had in person talk with Paul Kocialkowski and I have concluded that > >>>>> this is the best solution. > >>>>> I believe I should rename it to st7701s due to the hardware changes. I > >>>>> would like to create V5 patch with driver renamed to st7701s. > >>>>> Please let me know if you agree / disagree. > >>>> > >>>> If I recall it right, the ST7701 and ST7701S are basically the same > >>>> chip, aren't they ? > >>> > >>> I'm currently exploring all the differences. There aren't a lot of > >>> differences, but there are some. > >>> So far I can see that default register values are different, new > >>> previously unused registers are now used and there has been some > >>> reordering of how info is placed in registers [1] (data bits are in > >>> different order). Moreover, instructions to some commands have been > >>> changed and meaning of what data bits mean [2][3]. Also, new features > >>> have been added [2]; there is now PCLKS 3 for example. > >>> > >>> You can see few differences in following images. Same images were > >>> attached in this mail: > >>> [1] https://ibb.co/NmgbZmy - GAMACTRL_st7701.png > >>> [2] https://ibb.co/G79y235 - PCLKS2.png > >> > >> Ouch. I wonder if this is still something that can be abstracted out > >> with some helper accessor functions like: > >> > >> if (model == ST7701) > >> write something > >> else > >> write the other layout > >> > >> Or whether it makes sense to outright have a separate driver. The later > >> would introduce duplication, but maybe that much duplication is OK. > > > > I would like to create new driver because panel-st7701 seems to be > > outdated and is using non-standard macro (ST7701_WRITE() > > There is no such macro: > > $ git grep ST7701_WRITE drivers/gpu/drm/panel/ | wc -l > 0 > > There never was such a macro used in the driver either, are you sure you > are not using some hacked up patched downstream fork of the driver ? I meant ST7701_DSI() macro; It can be replaced with mipi_dsi_generic_write_seq from kernel 6.3. Sorry for the confusion. > > $ git log -p next/master -- > drivers/gpu/drm/panel/panel-sitronix-st7701.c | grep ST7701_WRITE | wc -l > 0 > > > ) and for me > > it is crashing kernel 5.15. > > Have you based all the aforementioned discussion and argumentation on > year and half old Linux 5.15.y code base too ? > > If so, you are missing many patches: > > $ git log --oneline --no-merges v5.15..next/master -- > drivers/gpu/drm/panel/panel-sitronix-st7701.c > 5a2854e577dc2 drm: panel: Add orientation support for st7701 > e89838968ee44 drm: panel: Add Elida KD50T048A to Sitronix ST7701 driver > c62102165dd79 drm/panel/panel-sitronix-st7701: Remove panel on DSI > attach failure > 49ee766b364ed drm/panel/panel-sitronix-st7701: Clean up CMDnBKx selection > c1cdee9b685a1 drm/panel/panel-sitronix-st7701: Fix RTNI calculation > 57b2efce45ef5 drm/panel/panel-sitronix-st7701: Add Densitron > DMT028VGHMCMI-1A TFT > 42542c7904cf2 drm/panel/panel-sitronix-st7701: Split GIP and init sequences > 83b7a8e7e88e7 drm/panel/panel-sitronix-st7701: Parametrize voltage and > timing > de2b4917843cd drm/panel/panel-sitronix-st7701: Infer horizontal pixel > count from TFT mode > 82f9cee25598a drm/panel/panel-sitronix-st7701: Adjust porch control > bitfield name > 1ba85119afb5e drm/panel/panel-sitronix-st7701: Infer vertical line count > from TFT mode > 779c84fea3dbd drm/panel/panel-sitronix-st7701: Make gamma correction TFT > specific > 7fa8e07128ed6 drm/panel/panel-sitronix-st7701: Make voltage supplies > common to ST7701 > a6c225be3da7e drm/panel/panel-sitronix-st7701: Enable DSI burst mode, > LPM, non-continuous clock > 6f481afe220d3 drm/panel/panel-sitronix-st7701: Make DSI mode flags > common to ST7701 > 79abca2b39900 drm/mipi-dsi: Make remove callback return void I will try backporting those patches to 5.15 and applying them to see whether it will then work with initialization sequences provided in this merge request just to be sure not to have duplication. We are still working on transitioning to newer kernel so for the time being I'm using mostly 5.15. On 5.15 kernel I have following kernel panic only with st7701 from the panel drivers I have tried: [ 20.255322] Kernel panic - not syncing: Asynchronous SError Interrupt [ 20.255326] CPU: 1 PID: 36 Comm: kworker/1:1 Tainted: G O 5.15.77-5.15.77-2.1.0 #1 [ 20.255330] Hardware name: XXX i.MX8XX board:XXX (DT) [ 20.255333] Workqueue: events fbcon_register_existing_fbs [ 20.255339] Call trace: [ 20.255340] dump_backtrace+0x0/0x19c [ 20.255349] show_stack+0x18/0x70 [ 20.255354] dump_stack_lvl+0x68/0x84 [ 20.255360] dump_stack+0x18/0x34 [ 20.255365] panic+0x15c/0x308 [ 20.255370] nmi_panic+0x8c/0x90 [ 20.255376] arm64_serror_panic+0x6c/0x7c [ 20.255382] do_serror+0x58/0x5c [ 20.255388] el1h_64_error_handler+0x30/0x50 [ 20.255395] el1h_64_error+0x78/0x7c [ 20.255399] _raw_spin_unlock_irq+0x18/0x50 [ 20.255404] sec_mipi_dsim_host_transfer+0x1a4/0x460 [ 20.255411] mipi_dsi_device_transfer+0x44/0x60 [ 20.255418] mipi_dsi_dcs_write_buffer+0x64/0xa0 [ 20.255424] st7701_init_sequence+0x2b0/0x47c [ 20.255431] st7701_prepare+0x54/0x70 [ 20.255436] drm_panel_prepare+0x28/0x40 [ 20.255431] st7701_prepare+0x54/0x70 [ 20.255436] drm_panel_prepare+0x28/0x40 [ 20.255441] sec_mipi_dsim_bridge_atomic_enable+0x308/0x520 [ 20.255446] drm_atomic_bridge_chain_enable+0x54/0xd0 [ 20.255453] drm_atomic_helper_commit_modeset_enables+0x13c/0x24c [ 20.255458] lcdif_drm_atomic_commit_tail+0x30/0x70 [ 20.255464] commit_tail+0xa0/0x180 [ 20.255468] drm_atomic_helper_commit+0x160/0x370 [ 20.255473] drm_atomic_commit+0x4c/0x60 [ 20.255477] drm_client_modeset_commit_atomic+0x1c8/0x260 [ 20.255485] drm_client_modeset_commit_locked+0x5c/0x1a0 [ 20.255491] drm_client_modeset_commit+0x30/0x60 [ 20.255496] drm_fb_helper_set_par+0xc8/0x120 [ 20.255504] fbcon_init+0x3c8/0x510 [ 20.255508] visual_init+0xb4/0x104 [ 20.255513] do_bind_con_driver.isra.0+0x1c4/0x394 [ 20.255518] do_take_over_console+0x144/0x1fc [ 20.255523] do_fbcon_takeover+0x6c/0xe0 [ 20.255530] fbcon_fb_registered+0xf4/0x140 [ 20.255534] fbcon_register_existing_fbs+0x48/0x64 [ 20.255539] process_one_work+0x1d0/0x354 [ 20.255545] worker_thread+0x13c/0x470 [ 20.255550] kthread+0x150/0x160 [ 20.255556] ret_from_fork+0x10/0x20 [ 20.255563] SMP: stopping secondary CPUs [ 20.255571] Kernel Offset: 0x80000 from 0xffff800008000000 [ 20.255574] PHYS_OFFSET: 0x40000000 [ 20.255563] SMP: stopping secondary CPUs [ 20.255571] Kernel Offset: 0x80000 from 0xffff800008000000 [ 20.255574] PHYS_OFFSET: 0x40000000 [ 20.255575] CPU features: 0x00002401,20000842 [ 20.255579] Memory Limit: none Raydium driver, for example, doesn't crash. It even almost works with few modifications (distorted screen). > > > Does anyone have similar issues with it? > > No, I am using it in production. Okay, then it might be something with my setup, I will check it out. Thank you for your time, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-12 15:10 ` Paulo Pavacic @ 2023-07-12 17:25 ` Marek Vasut 2023-09-05 8:32 ` Paulo 0 siblings, 1 reply; 22+ messages in thread From: Marek Vasut @ 2023-07-12 17:25 UTC (permalink / raw) To: Paulo Pavacic Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk On 7/12/23 17:10, Paulo Pavacic wrote: Hi, [...] >>>> Or whether it makes sense to outright have a separate driver. The later >>>> would introduce duplication, but maybe that much duplication is OK. >>> >>> I would like to create new driver because panel-st7701 seems to be >>> outdated and is using non-standard macro (ST7701_WRITE() >> >> There is no such macro: >> >> $ git grep ST7701_WRITE drivers/gpu/drm/panel/ | wc -l >> 0 >> >> There never was such a macro used in the driver either, are you sure you >> are not using some hacked up patched downstream fork of the driver ? > > I meant ST7701_DSI() macro; It can be replaced with > mipi_dsi_generic_write_seq from kernel 6.3. Sorry for the confusion. OK >> $ git log -p next/master -- >> drivers/gpu/drm/panel/panel-sitronix-st7701.c | grep ST7701_WRITE | wc -l >> 0 >> >>> ) and for me >>> it is crashing kernel 5.15. >> >> Have you based all the aforementioned discussion and argumentation on >> year and half old Linux 5.15.y code base too ? >> >> If so, you are missing many patches: >> >> $ git log --oneline --no-merges v5.15..next/master -- >> drivers/gpu/drm/panel/panel-sitronix-st7701.c >> 5a2854e577dc2 drm: panel: Add orientation support for st7701 >> e89838968ee44 drm: panel: Add Elida KD50T048A to Sitronix ST7701 driver >> c62102165dd79 drm/panel/panel-sitronix-st7701: Remove panel on DSI >> attach failure >> 49ee766b364ed drm/panel/panel-sitronix-st7701: Clean up CMDnBKx selection >> c1cdee9b685a1 drm/panel/panel-sitronix-st7701: Fix RTNI calculation >> 57b2efce45ef5 drm/panel/panel-sitronix-st7701: Add Densitron >> DMT028VGHMCMI-1A TFT >> 42542c7904cf2 drm/panel/panel-sitronix-st7701: Split GIP and init sequences >> 83b7a8e7e88e7 drm/panel/panel-sitronix-st7701: Parametrize voltage and >> timing >> de2b4917843cd drm/panel/panel-sitronix-st7701: Infer horizontal pixel >> count from TFT mode >> 82f9cee25598a drm/panel/panel-sitronix-st7701: Adjust porch control >> bitfield name >> 1ba85119afb5e drm/panel/panel-sitronix-st7701: Infer vertical line count >> from TFT mode >> 779c84fea3dbd drm/panel/panel-sitronix-st7701: Make gamma correction TFT >> specific >> 7fa8e07128ed6 drm/panel/panel-sitronix-st7701: Make voltage supplies >> common to ST7701 >> a6c225be3da7e drm/panel/panel-sitronix-st7701: Enable DSI burst mode, >> LPM, non-continuous clock >> 6f481afe220d3 drm/panel/panel-sitronix-st7701: Make DSI mode flags >> common to ST7701 >> 79abca2b39900 drm/mipi-dsi: Make remove callback return void > > I will try backporting those patches to 5.15 and applying them to see > whether it will then work with initialization sequences provided in > this merge request just to be sure not to have duplication. We are > still working on transitioning to newer kernel so for the time being > I'm using mostly 5.15. > > On 5.15 kernel I have following kernel panic only with st7701 from the > panel drivers I have tried: > > [ 20.255322] Kernel panic - not syncing: Asynchronous SError Interrupt > [ 20.255326] CPU: 1 PID: 36 Comm: kworker/1:1 Tainted: G O > 5.15.77-5.15.77-2.1.0 #1 The latest 5.15.y is 5.15.120 , can you re-test on that version ? > [ 20.255330] Hardware name: XXX i.MX8XX board:XXX (DT) Is this some NXP downstream kernel fork with thousands of extra patches? The version string 2.1.0 looks very much like NXP versioning scheme ... [...] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-12 17:25 ` Marek Vasut @ 2023-09-05 8:32 ` Paulo 0 siblings, 0 replies; 22+ messages in thread From: Paulo @ 2023-09-05 8:32 UTC (permalink / raw) To: Marek Vasut Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk Hi, > The latest 5.15.y is 5.15.120 , can you re-test on that version ? > unfortunately it seems that I won't be able to do testing with 5.15.120. I won't have access to hardware any more. > > [ 20.255330] Hardware name: XXX i.MX8XX board:XXX (DT) > > Is this some NXP downstream kernel fork with thousands of extra patches? > The version string 2.1.0 looks very much like NXP versioning scheme ... Yes, this is NXP fork of the kernel. > > [...] Best regards, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-07-06 15:26 ` Marek Vasut 2023-07-07 15:26 ` Paulo Pavacic @ 2023-07-08 7:11 ` Paulo Pavačić 1 sibling, 0 replies; 22+ messages in thread From: Paulo Pavačić @ 2023-07-08 7:11 UTC (permalink / raw) To: Marek Vasut Cc: neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, linux-kernel, dri-devel, robh+dt, Jagan Teki, sam, Maya Matuszczyk Hello Marek, Jul 6, 2023 5:26:15 PM Marek Vasut <marex@denx.de>: > On 7/6/23 17:18, Paulo Pavacic wrote: >> Hello Linus, >> čet, 22. lip 2023. u 10:22 Linus Walleij <linus.walleij@linaro.org> napisao je: >>> >>> On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: >>> >>>> A lot of modifications to st7701 are required. I believe it would >>>> result in a driver that doesn't look or work the same. e.g compare >>>> delays between initialization sequences of panel-fannal-c3004 and >>>> panel-st7701. I think it would be optimal to create st7701s driver and >>>> have special handling for st7701s panels. If there was a flag for >>>> whether panel is st7701 or st7701s it would end up looking like a >>>> mess. >>> >>> What matters is if the original authors of the old st7701 driver are >>> around and reviewing and testing patches at all. What we need is >>> active maintainers. (Added Jagan, Marek & Maya). >>> >>> I buy the reasoning that the st7701s is perhaps substantially different >>> from st7701. >>> >>> If st7701s is very different then I suppose it needs a separate driver, >>> then all we need to to name the driver properly, i.e. >>> panel-sitronix-st7701s.c. >> I had in person talk with Paul Kocialkowski and I have concluded that >> this is the best solution. >> I believe I should rename it to st7701s due to the hardware changes. I >> would like to create V5 patch with driver renamed to st7701s. >> Please let me know if you agree / disagree. > > If I recall it right, the ST7701 and ST7701S are basically the same chip, aren't they ? I'm currently exploring all the differences. There aren't a lot of them, but there are some. So far I can see that default register values are different, previously unused registers are now used and there has been some reordering of how info is placed in registers [1] (return value for some commands is different). E.g AJ1N[1:0] has been moved from B102h to B101h [1] Moreover, instructions to some commands have been changed as well as meaning of what data bits mean [2][3]. Also, new features have been added [2]; there is now PCLKS 3 for example. You can see few differences in following images: [1] https://ibb.co/NmgbZmy - GAMACTRL_st7701.png [2] https://ibb.co/G79y235 - PCLKS2.png P.S. this is second time I'm trying to send this e-mail so some of you might have received e-mail with the same text twice Thank you for your time, Paulo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel 2023-06-22 8:22 ` Linus Walleij 2023-07-06 15:18 ` Paulo Pavacic @ 2023-07-08 8:22 ` Jagan Teki 1 sibling, 0 replies; 22+ messages in thread From: Jagan Teki @ 2023-07-08 8:22 UTC (permalink / raw) To: Linus Walleij, Paulo Pavacic Cc: Marek Vasut, neil.armstrong, conor+dt, krzysztof.kozlowski+dt, devicetree, sam, linux-kernel, dri-devel, robh+dt, Jagan Teki, Maya Matuszczyk On Thu, 22 Jun 2023 at 13:52, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Jun 21, 2023 at 5:09 PM Paulo Pavacic <pavacic.p@gmail.com> wrote: > > > A lot of modifications to st7701 are required. I believe it would > > result in a driver that doesn't look or work the same. e.g compare > > delays between initialization sequences of panel-fannal-c3004 and > > panel-st7701. I think it would be optimal to create st7701s driver and > > have special handling for st7701s panels. If there was a flag for > > whether panel is st7701 or st7701s it would end up looking like a > > mess. > > What matters is if the original authors of the old st7701 driver are > around and reviewing and testing patches at all. What we need is > active maintainers. (Added Jagan, Marek & Maya). > > I buy the reasoning that the st7701s is perhaps substantially different > from st7701. > > If st7701s is very different then I suppose it needs a separate driver, > then all we need to to name the driver properly, i.e. > panel-sitronix-st7701s.c. I agree with what Linus mentioned. 1. If the panel is designed on top of ST7701 then add driver data on the existing panel-st7701 driver with this panel. 2. If the panel is designed on top of ST7701S - ST7701 and ST7701S are completely different in terms of the command set and init sequence then add panel-sitronix-st7701s.c 3. If the panel is designed on top ST7701S and if the commands set is the same as ST7701 but the init sequence is different then it is possible to use the existing st7701 driver with the init sequence as in driver data. Thanks, Jagan. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-09-05 8:32 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-07 15:11 [PATCH v4 0/3] drm/panel: add fannal c3004 panel Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 1/3] dt-bindings: add fannal vendor prefix Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 2/3] dt-bindings: display: panel: add fannal,c3004 Paulo Pavacic 2023-06-07 15:11 ` [PATCH v4 3/3] drm/panel-fannal-c3004: Add fannal c3004 DSI panel Paulo Pavacic 2023-06-15 19:54 ` Linus Walleij 2023-06-16 9:57 ` Paulo Pavacic 2023-06-16 11:43 ` Linus Walleij 2023-06-16 12:31 ` Paulo Pavacic 2023-06-16 12:52 ` Linus Walleij 2023-06-21 15:08 ` Paulo Pavacic 2023-06-22 8:22 ` Linus Walleij 2023-07-06 15:18 ` Paulo Pavacic 2023-07-06 15:26 ` Marek Vasut 2023-07-07 15:26 ` Paulo Pavacic 2023-07-08 11:07 ` Marek Vasut 2023-07-12 12:07 ` Paulo Pavacic 2023-07-12 13:42 ` Marek Vasut 2023-07-12 15:10 ` Paulo Pavacic 2023-07-12 17:25 ` Marek Vasut 2023-09-05 8:32 ` Paulo 2023-07-08 7:11 ` Paulo Pavačić 2023-07-08 8:22 ` Jagan Teki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).
![[-- Attachment #2: PCLKS2.png --] [-- Type: image/png, Size: 173528 bytes --]](../../CAO9szn20RY3uBDceyUJ1S+gb=FN8Hd5qqMfOSbitHFyFCZ+iLg@mail.gmail.com/2-PCLKS2.png){kind=link}
![[-- Attachment #3: GAMCTRL_st7701.png --] [-- Type: image/png, Size: 313421 bytes --]](../../CAO9szn20RY3uBDceyUJ1S+gb=FN8Hd5qqMfOSbitHFyFCZ+iLg@mail.gmail.com/3-GAMCTRL_st7701.png){kind=link}