2020-05-01 20:50:26

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 0/2] Add support for TM5P5 NT35596 video mode panel

I am aware of the fact that this is probably not the correct
naming of this panel, yet I am unable to retrieve any additional
information about it, as it is used in a smartphone to which no
schematics are released.

The driver has been generated with the help of
linux-mdss-dsi-panel-driver-generator [1] and works perfectly
on a Asus Zenfone 2 Laser Z00T smartphone, including brighness
control and switching on/off.

[1] https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator

Konrad Dybcio (2):
drivers: drm: panel: Add TM5P5 NT35596 panel driver
dt-bindings: display: Document TM5P5 NT35596 panel compatible

.../bindings/display/panel/tm5p5,nt35596.txt | 7 +
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 ++++++++++++++++++
4 files changed, 383 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c

--
2.26.1


2020-05-01 20:52:25

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 1/2] drivers: drm: panel: Add TM5P5 NT35596 panel driver

This adds support for TMP5P5 NT35596 1080x1920 video
mode panel that can be found on some Asus Zenfone 2
Laser (Z00T) devices.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 ++++++++++++++++++++
3 files changed, 376 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a1723c1b5fbf8..6ff892334ac4b 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -401,6 +401,15 @@ config DRM_PANEL_SONY_ACX565AKM
Say Y here if you want to enable support for the Sony ACX565AKM
800x600 3.5" panel (found on the Nokia N900).

+config DRM_PANEL_TM5P5_NT35596
+ tristate "TM5P5 NT35596 panel"
+ depends on GPIOLIB && OF
+ depends on DRM_MIPI_DSI
+ help
+ Say Y here if you want to enable support for the TMP5P5
+ NT35596 1080x1920 video mode panel as found in some Asus
+ Zenfone 2 Laser Z00T devices.
+
config DRM_PANEL_TPO_TD028TTEC1
tristate "Toppoly (TPO) TD028TTEC1 panel driver"
depends on OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 96a883cd66305..4fc7e00b18502 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
obj-$(CONFIG_DRM_PANEL_SONY_ACX424AKP) += panel-sony-acx424akp.o
obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
+obj-$(CONFIG_DRM_PANEL_TM5P5_NT35596) += panel-tm5p5-nt35596.o
obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
diff --git a/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
new file mode 100644
index 0000000000000..c361ab76812b8
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+struct tm5p5_nt35596 {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+ bool prepared;
+};
+
+static inline struct tm5p5_nt35596 *to_tm5p5_nt35596(struct drm_panel *panel)
+{
+ return container_of(panel, struct tm5p5_nt35596, panel);
+}
+
+#define dsi_generic_write_seq(dsi, seq...) do { \
+ static const u8 d[] = { seq }; \
+ int ret; \
+ ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \
+ if (ret < 0) \
+ return ret; \
+ } while (0)
+
+#define dsi_dcs_write_seq(dsi, seq...) do { \
+ static const u8 d[] = { seq }; \
+ int ret; \
+ ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
+ if (ret < 0) \
+ return ret; \
+ } while (0)
+
+static void tm5p5_nt35596_reset(struct tm5p5_nt35596 *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(15000, 16000);
+}
+
+static int tm5p5_nt35596_on(struct tm5p5_nt35596 *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+
+ dsi_generic_write_seq(dsi, 0xff, 0x05);
+ dsi_generic_write_seq(dsi, 0xfb, 0x01);
+ dsi_generic_write_seq(dsi, 0xc5, 0x31);
+ dsi_generic_write_seq(dsi, 0xff, 0x04);
+ dsi_generic_write_seq(dsi, 0x01, 0x84);
+ dsi_generic_write_seq(dsi, 0x05, 0x25);
+ dsi_generic_write_seq(dsi, 0x06, 0x01);
+ dsi_generic_write_seq(dsi, 0x07, 0x20);
+ dsi_generic_write_seq(dsi, 0x08, 0x06);
+ dsi_generic_write_seq(dsi, 0x09, 0x08);
+ dsi_generic_write_seq(dsi, 0x0a, 0x10);
+ dsi_generic_write_seq(dsi, 0x0b, 0x10);
+ dsi_generic_write_seq(dsi, 0x0c, 0x10);
+ dsi_generic_write_seq(dsi, 0x0d, 0x14);
+ dsi_generic_write_seq(dsi, 0x0e, 0x14);
+ dsi_generic_write_seq(dsi, 0x0f, 0x14);
+ dsi_generic_write_seq(dsi, 0x10, 0x14);
+ dsi_generic_write_seq(dsi, 0x11, 0x14);
+ dsi_generic_write_seq(dsi, 0x12, 0x14);
+ dsi_generic_write_seq(dsi, 0x17, 0xf3);
+ dsi_generic_write_seq(dsi, 0x18, 0xc0);
+ dsi_generic_write_seq(dsi, 0x19, 0xc0);
+ dsi_generic_write_seq(dsi, 0x1a, 0xc0);
+ dsi_generic_write_seq(dsi, 0x1b, 0xb3);
+ dsi_generic_write_seq(dsi, 0x1c, 0xb3);
+ dsi_generic_write_seq(dsi, 0x1d, 0xb3);
+ dsi_generic_write_seq(dsi, 0x1e, 0xb3);
+ dsi_generic_write_seq(dsi, 0x1f, 0xb3);
+ dsi_generic_write_seq(dsi, 0x20, 0xb3);
+ dsi_generic_write_seq(dsi, 0xfb, 0x01);
+ dsi_generic_write_seq(dsi, 0xff, 0x00);
+ dsi_generic_write_seq(dsi, 0xfb, 0x01);
+ dsi_generic_write_seq(dsi, 0x35, 0x01);
+ dsi_generic_write_seq(dsi, 0xd3, 0x06);
+ dsi_generic_write_seq(dsi, 0xd4, 0x04);
+ dsi_generic_write_seq(dsi, 0x5e, 0x0d);
+ dsi_generic_write_seq(dsi, 0x11, 0x00);
+ msleep(100);
+ dsi_generic_write_seq(dsi, 0x29, 0x00);
+ dsi_generic_write_seq(dsi, 0x53, 0x24);
+
+ return 0;
+}
+
+static int tm5p5_nt35596_off(struct tm5p5_nt35596 *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct device *dev = &dsi->dev;
+ int ret;
+
+ ret = mipi_dsi_dcs_set_display_off(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display off: %d\n", ret);
+ return ret;
+ }
+ msleep(60);
+
+ ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+ return ret;
+ }
+
+ dsi_dcs_write_seq(dsi, 0x4f, 0x01);
+
+ return 0;
+}
+
+static int tm5p5_nt35596_prepare(struct drm_panel *panel)
+{
+ struct tm5p5_nt35596 *ctx = to_tm5p5_nt35596(panel);
+ struct device *dev = &ctx->dsi->dev;
+ int ret;
+
+ if (ctx->prepared)
+ return 0;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enable regulators: %d\n", ret);
+ return ret;
+ }
+
+ tm5p5_nt35596_reset(ctx);
+
+ ret = tm5p5_nt35596_on(ctx);
+ if (ret < 0) {
+ dev_err(dev, "Failed to initialize panel: %d\n", ret);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+ return ret;
+ }
+
+ ctx->prepared = true;
+ return 0;
+}
+
+static int tm5p5_nt35596_unprepare(struct drm_panel *panel)
+{
+ struct tm5p5_nt35596 *ctx = to_tm5p5_nt35596(panel);
+ struct device *dev = &ctx->dsi->dev;
+ int ret;
+
+ if (!ctx->prepared)
+ return 0;
+
+ ret = tm5p5_nt35596_off(ctx);
+ if (ret < 0)
+ dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
+
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+
+ ctx->prepared = false;
+ return 0;
+}
+
+static const struct drm_display_mode tm5p5_nt35596_mode = {
+ .clock = (1080 + 100 + 8 + 16) * (1920 + 4 + 2 + 4) * 60 / 1000,
+ .hdisplay = 1080,
+ .hsync_start = 1080 + 100,
+ .hsync_end = 1080 + 100 + 8,
+ .htotal = 1080 + 100 + 8 + 16,
+ .vdisplay = 1920,
+ .vsync_start = 1920 + 4,
+ .vsync_end = 1920 + 4 + 2,
+ .vtotal = 1920 + 4 + 2 + 4,
+ .vrefresh = 60,
+ .width_mm = 68,
+ .height_mm = 121,
+};
+
+static int tm5p5_nt35596_get_modes(struct drm_panel *panel,
+ struct drm_connector *connector)
+{
+ struct drm_display_mode *mode;
+
+ mode = drm_mode_duplicate(connector->dev, &tm5p5_nt35596_mode);
+ if (!mode)
+ return -ENOMEM;
+
+ drm_mode_set_name(mode);
+
+ mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+ connector->display_info.width_mm = mode->width_mm;
+ connector->display_info.height_mm = mode->height_mm;
+ drm_mode_probed_add(connector, mode);
+
+ return 1;
+}
+
+static const struct drm_panel_funcs tm5p5_nt35596_panel_funcs = {
+ .prepare = tm5p5_nt35596_prepare,
+ .unprepare = tm5p5_nt35596_unprepare,
+ .get_modes = tm5p5_nt35596_get_modes,
+};
+
+static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness = bl->props.brightness;
+ int ret;
+
+ if (bl->props.power != FB_BLANK_UNBLANK ||
+ bl->props.fb_blank != FB_BLANK_UNBLANK ||
+ bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
+ brightness = 0;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return 0;
+}
+
+// TODO: Check if /sys/class/backlight/.../actual_brightness actually returns
+// correct values. If not, remove this function.
+static int tm5p5_nt35596_bl_get_brightness(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness = bl->props.brightness;
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return brightness & 0xff;
+}
+
+static const struct backlight_ops tm5p5_nt35596_bl_ops = {
+ .update_status = tm5p5_nt35596_bl_update_status,
+ .get_brightness = tm5p5_nt35596_bl_get_brightness,
+};
+
+static struct backlight_device *
+tm5p5_nt35596_create_backlight(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct backlight_properties props = {
+ .type = BACKLIGHT_RAW,
+ .brightness = 255,
+ .max_brightness = 255,
+ };
+
+ return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+ &tm5p5_nt35596_bl_ops, &props);
+}
+
+static int tm5p5_nt35596_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct tm5p5_nt35596 *ctx;
+ int ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->supplies[0].supply = "vdd";
+ ctx->supplies[1].supply = "vddio";
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+ ctx->supplies);
+ if (ret < 0) {
+ dev_err(dev, "Failed to get regulators: %d\n", ret);
+ return ret;
+ }
+
+ ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(ctx->reset_gpio)) {
+ ret = PTR_ERR(ctx->reset_gpio);
+ dev_err(dev, "Failed to get reset-gpios: %d\n", ret);
+ return ret;
+ }
+
+ ctx->dsi = dsi;
+ mipi_dsi_set_drvdata(dsi, ctx);
+
+ dsi->lanes = 4;
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_EOT_PACKET |
+ MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
+
+ drm_panel_init(&ctx->panel, dev, &tm5p5_nt35596_panel_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+
+ ctx->panel.backlight = tm5p5_nt35596_create_backlight(dsi);
+ if (IS_ERR(ctx->panel.backlight)) {
+ ret = PTR_ERR(ctx->panel.backlight);
+ dev_err(dev, "Failed to create backlight: %d\n", ret);
+ return ret;
+ }
+
+ ret = drm_panel_add(&ctx->panel);
+ if (ret < 0) {
+ dev_err(dev, "Failed to add panel: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to attach to DSI host: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int tm5p5_nt35596_remove(struct mipi_dsi_device *dsi)
+{
+ struct tm5p5_nt35596 *ctx = mipi_dsi_get_drvdata(dsi);
+ int ret;
+
+ ret = mipi_dsi_detach(dsi);
+ if (ret < 0)
+ dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+ drm_panel_remove(&ctx->panel);
+
+ return 0;
+}
+
+static const struct of_device_id tm5p5_nt35596_of_match[] = {
+ { .compatible = "tm5p5,nt35596" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tm5p5_nt35596_of_match);
+
+static struct mipi_dsi_driver tm5p5_nt35596_driver = {
+ .probe = tm5p5_nt35596_probe,
+ .remove = tm5p5_nt35596_remove,
+ .driver = {
+ .name = "panel-tm5p5-nt35596",
+ .of_match_table = tm5p5_nt35596_of_match,
+ },
+};
+module_mipi_dsi_driver(tm5p5_nt35596_driver);
+
+MODULE_AUTHOR("Konrad Dybcio <[email protected]>");
+MODULE_DESCRIPTION("DRM driver for tm5p5 nt35596 1080p video mode dsi panel");
+MODULE_LICENSE("GPL v2");
--
2.26.1

2020-05-01 20:52:56

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 2/2] dt-bindings: display: Document TM5P5 NT35596 panel compatible

Signed-off-by: Konrad Dybcio <[email protected]>
---
.../devicetree/bindings/display/panel/tm5p5,nt35596.txt | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt

diff --git a/Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt b/Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
new file mode 100644
index 0000000000000..6be56983482bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
@@ -0,0 +1,7 @@
+TM5P5 NT35596 5.5" 1080×1920 LCD Panel
+
+Required properties:
+ - compatible: "tm5p5,nt35596"
+ - reset-gpios: GPIO spec for reset pin
+ - vdd-supply: VDD regulator
+ - vddio-supply: VDDIO regulator
--
2.26.1

2020-05-01 20:58:23

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: display: Document TM5P5 NT35596 panel compatible

Hi Konrad.

Thanks for the new panel binding.
But you need to redo this as panel bindings today must be in DT Schema
format (.yaml).
Please see other bindings in the same dir for examples.

Sam

On Fri, May 01, 2020 at 10:48:23PM +0200, Konrad Dybcio wrote:
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> .../devicetree/bindings/display/panel/tm5p5,nt35596.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
>
> diff --git a/Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt b/Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
> new file mode 100644
> index 0000000000000..6be56983482bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
> @@ -0,0 +1,7 @@
> +TM5P5 NT35596 5.5" 1080?1920 LCD Panel
> +
> +Required properties:
> + - compatible: "tm5p5,nt35596"
> + - reset-gpios: GPIO spec for reset pin
> + - vdd-supply: VDD regulator
> + - vddio-supply: VDDIO regulator
> --
> 2.26.1

2020-05-01 21:04:46

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 0/2] Add support for TM5P5 NT35596 video mode panel

Hi Konrad.

On Fri, May 01, 2020 at 10:48:21PM +0200, Konrad Dybcio wrote:
> I am aware of the fact that this is probably not the correct
> naming of this panel, yet I am unable to retrieve any additional
> information about it, as it is used in a smartphone to which no
> schematics are released.
>
> The driver has been generated with the help of
> linux-mdss-dsi-panel-driver-generator [1] and works perfectly
> on a Asus Zenfone 2 Laser Z00T smartphone, including brighness
> control and switching on/off.
>
> [1] https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator

Panle driver looks good.
Will take a closer look tomorrow.

Any chance you can work on the TODO in the driver so we can have that
resolved before we apply it?

Also for a v2 it would be perfect if you could work on top of
drm-misc-next.
There is at least one small fix needed to build that I spotted.

But wait until I get back on the driver patch before submitting a v2.

Sam

>
> Konrad Dybcio (2):
> drivers: drm: panel: Add TM5P5 NT35596 panel driver
> dt-bindings: display: Document TM5P5 NT35596 panel compatible
>
> .../bindings/display/panel/tm5p5,nt35596.txt | 7 +
> drivers/gpu/drm/panel/Kconfig | 9 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 ++++++++++++++++++
> 4 files changed, 383 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
> create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
>
> --
> 2.26.1

2020-05-01 21:34:16

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 0/2] Add support for TM5P5 NT35596 video mode panel

Very strange.. I just fetched drm-misc-next and had no issues building...

Thanks for your initial thoughts!

pt., 1 maj 2020 o 23:00 Sam Ravnborg <[email protected]> napisał(a):
>
> Hi Konrad.
>
> On Fri, May 01, 2020 at 10:48:21PM +0200, Konrad Dybcio wrote:
> > I am aware of the fact that this is probably not the correct
> > naming of this panel, yet I am unable to retrieve any additional
> > information about it, as it is used in a smartphone to which no
> > schematics are released.
> >
> > The driver has been generated with the help of
> > linux-mdss-dsi-panel-driver-generator [1] and works perfectly
> > on a Asus Zenfone 2 Laser Z00T smartphone, including brighness
> > control and switching on/off.
> >
> > [1] https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
>
> Panle driver looks good.
> Will take a closer look tomorrow.
>
> Any chance you can work on the TODO in the driver so we can have that
> resolved before we apply it?
>
> Also for a v2 it would be perfect if you could work on top of
> drm-misc-next.
> There is at least one small fix needed to build that I spotted.
>
> But wait until I get back on the driver patch before submitting a v2.
>
> Sam
>
> >
> > Konrad Dybcio (2):
> > drivers: drm: panel: Add TM5P5 NT35596 panel driver
> > dt-bindings: display: Document TM5P5 NT35596 panel compatible
> >
> > .../bindings/display/panel/tm5p5,nt35596.txt | 7 +
> > drivers/gpu/drm/panel/Kconfig | 9 +
> > drivers/gpu/drm/panel/Makefile | 1 +
> > drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 ++++++++++++++++++
> > 4 files changed, 383 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/panel/tm5p5,nt35596.txt
> > create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> >
> > --
> > 2.26.1

2020-05-02 07:11:57

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] drivers: drm: panel: Add TM5P5 NT35596 panel driver

Hi Konrad.

On Fri, May 01, 2020 at 10:48:22PM +0200, Konrad Dybcio wrote:
> This adds support for TMP5P5 NT35596 1080x1920 video
> mode panel that can be found on some Asus Zenfone 2
> Laser (Z00T) devices.

Very well-writen driver. Only a few small things in the following.

Sam

>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/panel/Kconfig | 9 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 ++++++++++++++++++++
> 3 files changed, 376 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index a1723c1b5fbf8..6ff892334ac4b 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -401,6 +401,15 @@ config DRM_PANEL_SONY_ACX565AKM
> Say Y here if you want to enable support for the Sony ACX565AKM
> 800x600 3.5" panel (found on the Nokia N900).
>
> +config DRM_PANEL_TM5P5_NT35596
> + tristate "TM5P5 NT35596 panel"
> + depends on GPIOLIB && OF
> + depends on DRM_MIPI_DSI
> + help
> + Say Y here if you want to enable support for the TMP5P5
> + NT35596 1080x1920 video mode panel as found in some Asus
> + Zenfone 2 Laser Z00T devices.
> +
> config DRM_PANEL_TPO_TD028TTEC1
> tristate "Toppoly (TPO) TD028TTEC1 panel driver"
> depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 96a883cd66305..4fc7e00b18502 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> obj-$(CONFIG_DRM_PANEL_SONY_ACX424AKP) += panel-sony-acx424akp.o
> obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
> +obj-$(CONFIG_DRM_PANEL_TM5P5_NT35596) += panel-tm5p5-nt35596.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
> obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> diff --git a/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> new file mode 100644
> index 0000000000000..c361ab76812b8
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +struct tm5p5_nt35596 {
> + struct drm_panel panel;
> + struct mipi_dsi_device *dsi;
> + struct regulator_bulk_data supplies[2];
> + struct gpio_desc *reset_gpio;
> + bool prepared;
> +};
> +
> +static inline struct tm5p5_nt35596 *to_tm5p5_nt35596(struct drm_panel *panel)
> +{
> + return container_of(panel, struct tm5p5_nt35596, panel);
> +}
> +
> +#define dsi_generic_write_seq(dsi, seq...) do { \
> + static const u8 d[] = { seq }; \
> + int ret; \
> + ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \
> + if (ret < 0) \
> + return ret; \
> + } while (0)
> +
> +#define dsi_dcs_write_seq(dsi, seq...) do { \
> + static const u8 d[] = { seq }; \
> + int ret; \
> + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
> + if (ret < 0) \
> + return ret; \
> + } while (0)
> +
> +static void tm5p5_nt35596_reset(struct tm5p5_nt35596 *ctx)
> +{
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> + usleep_range(1000, 2000);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> + usleep_range(1000, 2000);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> + usleep_range(15000, 16000);
> +}
> +
> +static int tm5p5_nt35596_on(struct tm5p5_nt35596 *ctx)
> +{
> + struct mipi_dsi_device *dsi = ctx->dsi;

Do you have any idea of what happens in the following?
I recall 0xff is explained in a few other drivers for example.

If you get an idea it would be nice with a few headlines.
> +
> + dsi_generic_write_seq(dsi, 0xff, 0x05);
> + dsi_generic_write_seq(dsi, 0xfb, 0x01);
> + dsi_generic_write_seq(dsi, 0xc5, 0x31);
> + dsi_generic_write_seq(dsi, 0xff, 0x04);
> + dsi_generic_write_seq(dsi, 0x01, 0x84);
> + dsi_generic_write_seq(dsi, 0x05, 0x25);
> + dsi_generic_write_seq(dsi, 0x06, 0x01);
> + dsi_generic_write_seq(dsi, 0x07, 0x20);
> + dsi_generic_write_seq(dsi, 0x08, 0x06);
> + dsi_generic_write_seq(dsi, 0x09, 0x08);
> + dsi_generic_write_seq(dsi, 0x0a, 0x10);
> + dsi_generic_write_seq(dsi, 0x0b, 0x10);
> + dsi_generic_write_seq(dsi, 0x0c, 0x10);
> + dsi_generic_write_seq(dsi, 0x0d, 0x14);
> + dsi_generic_write_seq(dsi, 0x0e, 0x14);
> + dsi_generic_write_seq(dsi, 0x0f, 0x14);
> + dsi_generic_write_seq(dsi, 0x10, 0x14);
> + dsi_generic_write_seq(dsi, 0x11, 0x14);
> + dsi_generic_write_seq(dsi, 0x12, 0x14);
> + dsi_generic_write_seq(dsi, 0x17, 0xf3);
> + dsi_generic_write_seq(dsi, 0x18, 0xc0);
> + dsi_generic_write_seq(dsi, 0x19, 0xc0);
> + dsi_generic_write_seq(dsi, 0x1a, 0xc0);
> + dsi_generic_write_seq(dsi, 0x1b, 0xb3);
> + dsi_generic_write_seq(dsi, 0x1c, 0xb3);
> + dsi_generic_write_seq(dsi, 0x1d, 0xb3);
> + dsi_generic_write_seq(dsi, 0x1e, 0xb3);
> + dsi_generic_write_seq(dsi, 0x1f, 0xb3);
> + dsi_generic_write_seq(dsi, 0x20, 0xb3);
> + dsi_generic_write_seq(dsi, 0xfb, 0x01);
> + dsi_generic_write_seq(dsi, 0xff, 0x00);
> + dsi_generic_write_seq(dsi, 0xfb, 0x01);
> + dsi_generic_write_seq(dsi, 0x35, 0x01);
> + dsi_generic_write_seq(dsi, 0xd3, 0x06);
> + dsi_generic_write_seq(dsi, 0xd4, 0x04);
> + dsi_generic_write_seq(dsi, 0x5e, 0x0d);
> + dsi_generic_write_seq(dsi, 0x11, 0x00);
> + msleep(100);
> + dsi_generic_write_seq(dsi, 0x29, 0x00);
> + dsi_generic_write_seq(dsi, 0x53, 0x24);
> +
> + return 0;
> +}
> +
> +static int tm5p5_nt35596_off(struct tm5p5_nt35596 *ctx)
> +{
> + struct mipi_dsi_device *dsi = ctx->dsi;
> + struct device *dev = &dsi->dev;
> + int ret;
> +
> + ret = mipi_dsi_dcs_set_display_off(dsi);
> + if (ret < 0) {
> + dev_err(dev, "Failed to set display off: %d\n", ret);
> + return ret;
> + }
> + msleep(60);
> +
> + ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> + if (ret < 0) {
> + dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
> + return ret;
> + }

I wonder why the panel do not need a:
mipi_dsi_dcs_exit_sleep_mode()
and
mipi_dsi_dcs_set_display_on()

in the tm5p5_nt35596_on() function.

Usually the prepare and unprepare steps are
somehow symmetrical.

> +
> + dsi_dcs_write_seq(dsi, 0x4f, 0x01);
> +
> + return 0;
> +}
> +
> +static int tm5p5_nt35596_prepare(struct drm_panel *panel)
> +{
> + struct tm5p5_nt35596 *ctx = to_tm5p5_nt35596(panel);
> + struct device *dev = &ctx->dsi->dev;
> + int ret;
> +
> + if (ctx->prepared)
> + return 0;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> + if (ret < 0) {
> + dev_err(dev, "Failed to enable regulators: %d\n", ret);
> + return ret;
> + }
> +
> + tm5p5_nt35596_reset(ctx);
> +
> + ret = tm5p5_nt35596_on(ctx);
> + if (ret < 0) {
> + dev_err(dev, "Failed to initialize panel: %d\n", ret);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
Line too long.

> + return ret;
> + }
> +
> + ctx->prepared = true;
> + return 0;
> +}
> +
> +static int tm5p5_nt35596_unprepare(struct drm_panel *panel)
> +{
> + struct tm5p5_nt35596 *ctx = to_tm5p5_nt35596(panel);
> + struct device *dev = &ctx->dsi->dev;
> + int ret;
> +
> + if (!ctx->prepared)
> + return 0;
> +
> + ret = tm5p5_nt35596_off(ctx);
> + if (ret < 0)
> + dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
> +
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +
> + ctx->prepared = false;
> + return 0;
> +}
> +
> +static const struct drm_display_mode tm5p5_nt35596_mode = {
> + .clock = (1080 + 100 + 8 + 16) * (1920 + 4 + 2 + 4) * 60 / 1000,
> + .hdisplay = 1080,
> + .hsync_start = 1080 + 100,
> + .hsync_end = 1080 + 100 + 8,
> + .htotal = 1080 + 100 + 8 + 16,
> + .vdisplay = 1920,
> + .vsync_start = 1920 + 4,
> + .vsync_end = 1920 + 4 + 2,
> + .vtotal = 1920 + 4 + 2 + 4,
> + .vrefresh = 60,
> + .width_mm = 68,
> + .height_mm = 121,
> +};
We have patches pending to remove vrefresh, which I thought
was already applied. Anyway, keep it here.
We will eaither remove it later or if the patches crosses
I will do it when applying this patch.

> +
> +static int tm5p5_nt35596_get_modes(struct drm_panel *panel,
> + struct drm_connector *connector)
> +{
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_duplicate(connector->dev, &tm5p5_nt35596_mode);
> + if (!mode)
> + return -ENOMEM;
> +
> + drm_mode_set_name(mode);
> +
> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> + connector->display_info.width_mm = mode->width_mm;
> + connector->display_info.height_mm = mode->height_mm;
> + drm_mode_probed_add(connector, mode);
> +
> + return 1;
> +}
> +
> +static const struct drm_panel_funcs tm5p5_nt35596_panel_funcs = {
> + .prepare = tm5p5_nt35596_prepare,
> + .unprepare = tm5p5_nt35596_unprepare,
> + .get_modes = tm5p5_nt35596_get_modes,
> +};
> +
> +static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
> +{
> + struct mipi_dsi_device *dsi = bl_get_data(bl);
> + u16 brightness = bl->props.brightness;
> + int ret;
> +
> + if (bl->props.power != FB_BLANK_UNBLANK ||
> + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> + brightness = 0;
> +
> + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> + ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> + if (ret < 0)
> + return ret;
> +
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + return 0;
> +}
> +
> +// TODO: Check if /sys/class/backlight/.../actual_brightness actually returns
> +// correct values. If not, remove this function.
Please resolve this TODO

> +static int tm5p5_nt35596_bl_get_brightness(struct backlight_device *bl)
> +{
> + struct mipi_dsi_device *dsi = bl_get_data(bl);
> + u16 brightness = bl->props.brightness;
> + int ret;
> +
> + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> + ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
> + if (ret < 0)
> + return ret;
> +
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + return brightness & 0xff;
> +}
> +
> +static const struct backlight_ops tm5p5_nt35596_bl_ops = {
> + .update_status = tm5p5_nt35596_bl_update_status,
> + .get_brightness = tm5p5_nt35596_bl_get_brightness,
> +};
> +
> +static struct backlight_device *
> +tm5p5_nt35596_create_backlight(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = &dsi->dev;
> + struct backlight_properties props = {
> + .type = BACKLIGHT_RAW,
> + .brightness = 255,
> + .max_brightness = 255,
> + };
Make props const.

> +
> + return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
> + &tm5p5_nt35596_bl_ops, &props);
> +}
> +
> +static int tm5p5_nt35596_probe(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = &dsi->dev;
> + struct tm5p5_nt35596 *ctx;
> + int ret;
> +
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->supplies[0].supply = "vdd";
> + ctx->supplies[1].supply = "vddio";
> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> + ctx->supplies);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get regulators: %d\n", ret);
> + return ret;
> + }
> +
> + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ctx->reset_gpio)) {
> + ret = PTR_ERR(ctx->reset_gpio);
> + dev_err(dev, "Failed to get reset-gpios: %d\n", ret);
> + return ret;
> + }
> +
> + ctx->dsi = dsi;
> + mipi_dsi_set_drvdata(dsi, ctx);
> +
> + dsi->lanes = 4;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> + MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_EOT_PACKET |
> + MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
> +
> + drm_panel_init(&ctx->panel, dev, &tm5p5_nt35596_panel_funcs,
> + DRM_MODE_CONNECTOR_DSI);
> +
> + ctx->panel.backlight = tm5p5_nt35596_create_backlight(dsi);
> + if (IS_ERR(ctx->panel.backlight)) {
> + ret = PTR_ERR(ctx->panel.backlight);
> + dev_err(dev, "Failed to create backlight: %d\n", ret);
> + return ret;
> + }
> +
> + ret = drm_panel_add(&ctx->panel);
> + if (ret < 0) {
> + dev_err(dev, "Failed to add panel: %d\n", ret);
> + return ret;
> + }
> +
> + ret = mipi_dsi_attach(dsi);
> + if (ret < 0) {
> + dev_err(dev, "Failed to attach to DSI host: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int tm5p5_nt35596_remove(struct mipi_dsi_device *dsi)
> +{
> + struct tm5p5_nt35596 *ctx = mipi_dsi_get_drvdata(dsi);
> + int ret;
> +
> + ret = mipi_dsi_detach(dsi);
> + if (ret < 0)
> + dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
Line too long I think

> +
> + drm_panel_remove(&ctx->panel);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id tm5p5_nt35596_of_match[] = {
> + { .compatible = "tm5p5,nt35596" },
vendor tm5p5 is not documented.

> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, tm5p5_nt35596_of_match);
> +
> +static struct mipi_dsi_driver tm5p5_nt35596_driver = {
> + .probe = tm5p5_nt35596_probe,
> + .remove = tm5p5_nt35596_remove,
> + .driver = {
> + .name = "panel-tm5p5-nt35596",
> + .of_match_table = tm5p5_nt35596_of_match,
> + },
> +};
> +module_mipi_dsi_driver(tm5p5_nt35596_driver);
> +
> +MODULE_AUTHOR("Konrad Dybcio <[email protected]>");
> +MODULE_DESCRIPTION("DRM driver for tm5p5 nt35596 1080p video mode dsi panel");
> +MODULE_LICENSE("GPL v2");
> --
> 2.26.1

2020-05-02 11:18:29

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] drivers: drm: panel: Add TM5P5 NT35596 panel driver

Hi Konrad.

On Sat, May 02, 2020 at 12:09:45PM +0200, Konrad Dybcio wrote:
> Hi,
>
> Thanks for your review. I'll send a v2 soon, however we need to solve the
> compat string issue first.
>
> How should I document tm5p5? I think it's rather some kind of a model no.
> along with the nt35596 IC than a vendor name.. Or should we call it
> something like "unknown,tm5p5-nt35596",

> perhaps "asus,z00t-tm5p5-n35596"
This is the best choice I think.
It is mounted inside an asus phone and we have no bette clue what vendor
it is.
Include the info you already provided in these mails in the binding,
so if anyone tries to dig deeper or have better knowledge one day we
have the original findings documented.

Searching a little reveals the the zenphone 2 laser uses a TruVivid
display - so seems to be asus manufactured.
So that only confirms that your suggestion is good.

Sam


> [1]?
>
> [1] Z00T is the model number of the smartphone that uses this panel
>
> Konrad
>
> On Sat, May 2, 2020, 09:05 Sam Ravnborg <[email protected]> wrote:
>
> > Hi Konrad.
> >
> > On Fri, May 01, 2020 at 10:48:22PM +0200, Konrad Dybcio wrote:
> > > This adds support for TMP5P5 NT35596 1080x1920 video
> > > mode panel that can be found on some Asus Zenfone 2
> > > Laser (Z00T) devices.
> >
> > Very well-writen driver. Only a few small things in the following.
> >
> > Sam
> >
> > >
> > > Signed-off-by: Konrad Dybcio <[email protected]>
> > > ---
> > > drivers/gpu/drm/panel/Kconfig | 9 +
> > > drivers/gpu/drm/panel/Makefile | 1 +
> > > drivers/gpu/drm/panel/panel-tm5p5-nt35596.c | 366 ++++++++++++++++++++
> > > 3 files changed, 376 insertions(+)
> > > create mode 100644 drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> > >
> > > diff --git a/drivers/gpu/drm/panel/Kconfig
> > b/drivers/gpu/drm/panel/Kconfig
> > > index a1723c1b5fbf8..6ff892334ac4b 100644
> > > --- a/drivers/gpu/drm/panel/Kconfig
> > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > @@ -401,6 +401,15 @@ config DRM_PANEL_SONY_ACX565AKM
> > > Say Y here if you want to enable support for the Sony ACX565AKM
> > > 800x600 3.5" panel (found on the Nokia N900).
> > >
> > > +config DRM_PANEL_TM5P5_NT35596
> > > + tristate "TM5P5 NT35596 panel"
> > > + depends on GPIOLIB && OF
> > > + depends on DRM_MIPI_DSI
> > > + help
> > > + Say Y here if you want to enable support for the TMP5P5
> > > + NT35596 1080x1920 video mode panel as found in some Asus
> > > + Zenfone 2 Laser Z00T devices.
> > > +
> > > config DRM_PANEL_TPO_TD028TTEC1
> > > tristate "Toppoly (TPO) TD028TTEC1 panel driver"
> > > depends on OF && SPI
> > > diff --git a/drivers/gpu/drm/panel/Makefile
> > b/drivers/gpu/drm/panel/Makefile
> > > index 96a883cd66305..4fc7e00b18502 100644
> > > --- a/drivers/gpu/drm/panel/Makefile
> > > +++ b/drivers/gpu/drm/panel/Makefile
> > > @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
> > panel-sitronix-st7701.o
> > > obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > > obj-$(CONFIG_DRM_PANEL_SONY_ACX424AKP) += panel-sony-acx424akp.o
> > > obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
> > > +obj-$(CONFIG_DRM_PANEL_TM5P5_NT35596) += panel-tm5p5-nt35596.o
> > > obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> > > obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
> > > obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> > > diff --git a/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> > b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> > > new file mode 100644
> > > index 0000000000000..c361ab76812b8
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/panel/panel-tm5p5-nt35596.c
> > > @@ -0,0 +1,366 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#include <linux/backlight.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/regulator/consumer.h>
> > > +
> > > +#include <drm/drm_mipi_dsi.h>
> > > +#include <drm/drm_modes.h>
> > > +#include <drm/drm_panel.h>
> > > +
> > > +struct tm5p5_nt35596 {
> > > + struct drm_panel panel;
> > > + struct mipi_dsi_device *dsi;
> > > + struct regulator_bulk_data supplies[2];
> > > + struct gpio_desc *reset_gpio;
> > > + bool prepared;
> > > +};
> > > +
> > > +static inline struct tm5p5_nt35596 *to_tm5p5_nt35596(struct drm_panel
> > *panel)
> > > +{
> > > + return container_of(panel, struct tm5p5_nt35596, panel);
> > > +}
> > > +
> > > +#define dsi_generic_write_seq(dsi, seq...) do {
> > \
> > > + static const u8 d[] = { seq }; \
> > > + int ret; \
> > > + ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \
> > > + if (ret < 0) \
> > > + return ret; \
> > > + } while (0)
> > > +
> > > +#define dsi_dcs_write_seq(dsi, seq...) do { \
> > > + static const u8 d[] = { seq }; \
> > > + int ret; \
> > > + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
> > > + if (ret < 0) \
> > > + return ret; \
> > > + } while (0)
> > > +
> > > +static void tm5p5_nt35596_reset(struct tm5p5_nt35596 *ctx)
> > > +{
> > > + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > > + usleep_range(1000, 2000);
> > > + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> > > + usleep_range(1000, 2000);
> > > + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > > + usleep_range(15000, 16000);
> > > +}
> > > +
> > > +static int tm5p5_nt35596_on(struct tm5p5_nt35596 *ctx)
> > > +{
> > > + struct mipi_dsi_device *dsi = ctx->dsi;
> >
> > Do you have any idea of what happens in the following?
> > I recall 0xff is explained in a few other drivers for example.
> >
> > If you get an idea it would be nice with a few headlines.
> > > +
> > > + dsi_generic_write_seq(dsi, 0xff, 0x05);
> > > + dsi_generic_write_seq(dsi, 0xfb, 0x01);
> > > + dsi_generic_write_seq(dsi, 0xc5, 0x31);
> > > + dsi_generic_write_seq(dsi, 0xff, 0x04);
> > > + dsi_generic_write_seq(dsi, 0x01, 0x84);
> > > + dsi_generic_write_seq(dsi, 0x05, 0x25);
> > > + dsi_generic_write_seq(dsi, 0x06, 0x01);
> > > + dsi_generic_write_seq(dsi, 0x07, 0x20);
> > > + dsi_generic_write_seq(dsi, 0x08, 0x06);
> > > + dsi_generic_write_seq(dsi, 0x09, 0x08);
> > > + dsi_generic_write_seq(dsi, 0x0a, 0x10);
> > > + dsi_generic_write_seq(dsi, 0x0b, 0x10);
> > > + dsi_generic_write_seq(dsi, 0x0c, 0x10);
> > > + dsi_generic_write_seq(dsi, 0x0d, 0x14);
> > > + dsi_generic_write_seq(dsi, 0x0e, 0x14);
> > > + dsi_generic_write_seq(dsi, 0x0f, 0x14);
> > > + dsi_generic_write_seq(dsi, 0x10, 0x14);
> > > + dsi_generic_write_seq(dsi, 0x11, 0x14);
> > > + dsi_generic_write_seq(dsi, 0x12, 0x14);
> > > + dsi_generic_write_seq(dsi, 0x17, 0xf3);
> > > + dsi_generic_write_seq(dsi, 0x18, 0xc0);
> > > + dsi_generic_write_seq(dsi, 0x19, 0xc0);
> > > + dsi_generic_write_seq(dsi, 0x1a, 0xc0);
> > > + dsi_generic_write_seq(dsi, 0x1b, 0xb3);
> > > + dsi_generic_write_seq(dsi, 0x1c, 0xb3);
> > > + dsi_generic_write_seq(dsi, 0x1d, 0xb3);
> > > + dsi_generic_write_seq(dsi, 0x1e, 0xb3);
> > > + dsi_generic_write_seq(dsi, 0x1f, 0xb3);
> > > + dsi_generic_write_seq(dsi, 0x20, 0xb3);
> > > + dsi_generic_write_seq(dsi, 0xfb, 0x01);
> > > + dsi_generic_write_seq(dsi, 0xff, 0x00);
> > > + dsi_generic_write_seq(dsi, 0xfb, 0x01);
> > > + dsi_generic_write_seq(dsi, 0x35, 0x01);
> > > + dsi_generic_write_seq(dsi, 0xd3, 0x06);
> > > + dsi_generic_write_seq(dsi, 0xd4, 0x04);
> > > + dsi_generic_write_seq(dsi, 0x5e, 0x0d);
> > > + dsi_generic_write_seq(dsi, 0x11, 0x00);
> > > + msleep(100);
> > > + dsi_generic_write_seq(dsi, 0x29, 0x00);
> > > + dsi_generic_write_seq(dsi, 0x53, 0x24);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tm5p5_nt35596_off(struct tm5p5_nt35596 *ctx)
> > > +{
> > > + struct mipi_dsi_device *dsi = ctx->dsi;
> > > + struct device *dev = &dsi->dev;
> > > + int ret;
> > > +
> > > + ret = mipi_dsi_dcs_set_display_off(dsi);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to set display off: %d\n", ret);
> > > + return ret;
> > > + }
> > > + msleep(60);
> > > +
> > > + ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
> > > + return ret;
> > > + }
> >
> > I wonder why the panel do not need a:
> > mipi_dsi_dcs_exit_sleep_mode()
> > and
> > mipi_dsi_dcs_set_display_on()
> >
> > in the tm5p5_nt35596_on() function.
> >
> > Usually the prepare and unprepare steps are
> > somehow symmetrical.
> >
> > > +
> > > + dsi_dcs_write_seq(dsi, 0x4f, 0x01);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tm5p5_nt35596_prepare(struct drm_panel *panel)
> > > +{
> > > + struct tm5p5_nt35596 *ctx = to_tm5p5_nt35596(panel);
> > > + struct device *dev = &ctx->dsi->dev;
> > > + int ret;
> > > +
> > > + if (ctx->prepared)
> > > + return 0;
> > > +
> > > + ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
> > ctx->supplies);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + tm5p5_nt35596_reset(ctx);
> > > +
> > > + ret = tm5p5_nt35596_on(ctx);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to initialize panel: %d\n", ret);
> > > + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> > > + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies),
> > ctx->supplies);
> > Line too long.
> >
> > > + return ret;
> > > + }
> > > +
> > > + ctx->prepared = true;
> > > + return 0;
> > > +}
> > > +
> > > +static int tm5p5_nt35596_unprepare(struct drm_panel *panel)
> > > +{
> > > + struct tm5p5_nt35596 *ctx = to_tm5p5_nt35596(panel);
> > > + struct device *dev = &ctx->dsi->dev;
> > > + int ret;
> > > +
> > > + if (!ctx->prepared)
> > > + return 0;
> > > +
> > > + ret = tm5p5_nt35596_off(ctx);
> > > + if (ret < 0)
> > > + dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
> > > +
> > > + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> > > + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > +
> > > + ctx->prepared = false;
> > > + return 0;
> > > +}
> > > +
> > > +static const struct drm_display_mode tm5p5_nt35596_mode = {
> > > + .clock = (1080 + 100 + 8 + 16) * (1920 + 4 + 2 + 4) * 60 / 1000,
> > > + .hdisplay = 1080,
> > > + .hsync_start = 1080 + 100,
> > > + .hsync_end = 1080 + 100 + 8,
> > > + .htotal = 1080 + 100 + 8 + 16,
> > > + .vdisplay = 1920,
> > > + .vsync_start = 1920 + 4,
> > > + .vsync_end = 1920 + 4 + 2,
> > > + .vtotal = 1920 + 4 + 2 + 4,
> > > + .vrefresh = 60,
> > > + .width_mm = 68,
> > > + .height_mm = 121,
> > > +};
> > We have patches pending to remove vrefresh, which I thought
> > was already applied. Anyway, keep it here.
> > We will eaither remove it later or if the patches crosses
> > I will do it when applying this patch.
> >
> > > +
> > > +static int tm5p5_nt35596_get_modes(struct drm_panel *panel,
> > > + struct drm_connector *connector)
> > > +{
> > > + struct drm_display_mode *mode;
> > > +
> > > + mode = drm_mode_duplicate(connector->dev, &tm5p5_nt35596_mode);
> > > + if (!mode)
> > > + return -ENOMEM;
> > > +
> > > + drm_mode_set_name(mode);
> > > +
> > > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> > > + connector->display_info.width_mm = mode->width_mm;
> > > + connector->display_info.height_mm = mode->height_mm;
> > > + drm_mode_probed_add(connector, mode);
> > > +
> > > + return 1;
> > > +}
> > > +
> > > +static const struct drm_panel_funcs tm5p5_nt35596_panel_funcs = {
> > > + .prepare = tm5p5_nt35596_prepare,
> > > + .unprepare = tm5p5_nt35596_unprepare,
> > > + .get_modes = tm5p5_nt35596_get_modes,
> > > +};
> > > +
> > > +static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
> > > +{
> > > + struct mipi_dsi_device *dsi = bl_get_data(bl);
> > > + u16 brightness = bl->props.brightness;
> > > + int ret;
> > > +
> > > + if (bl->props.power != FB_BLANK_UNBLANK ||
> > > + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> > > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > > + brightness = 0;
> > > +
> > > + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> > > +
> > > + ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +// TODO: Check if /sys/class/backlight/.../actual_brightness actually
> > returns
> > > +// correct values. If not, remove this function.
> > Please resolve this TODO
> >
> > > +static int tm5p5_nt35596_bl_get_brightness(struct backlight_device *bl)
> > > +{
> > > + struct mipi_dsi_device *dsi = bl_get_data(bl);
> > > + u16 brightness = bl->props.brightness;
> > > + int ret;
> > > +
> > > + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> > > +
> > > + ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> > > +
> > > + return brightness & 0xff;
> > > +}
> > > +
> > > +static const struct backlight_ops tm5p5_nt35596_bl_ops = {
> > > + .update_status = tm5p5_nt35596_bl_update_status,
> > > + .get_brightness = tm5p5_nt35596_bl_get_brightness,
> > > +};
> > > +
> > > +static struct backlight_device *
> > > +tm5p5_nt35596_create_backlight(struct mipi_dsi_device *dsi)
> > > +{
> > > + struct device *dev = &dsi->dev;
> > > + struct backlight_properties props = {
> > > + .type = BACKLIGHT_RAW,
> > > + .brightness = 255,
> > > + .max_brightness = 255,
> > > + };
> > Make props const.
> >
> > > +
> > > + return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
> > > + &tm5p5_nt35596_bl_ops,
> > &props);
> > > +}
> > > +
> > > +static int tm5p5_nt35596_probe(struct mipi_dsi_device *dsi)
> > > +{
> > > + struct device *dev = &dsi->dev;
> > > + struct tm5p5_nt35596 *ctx;
> > > + int ret;
> > > +
> > > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > > + if (!ctx)
> > > + return -ENOMEM;
> > > +
> > > + ctx->supplies[0].supply = "vdd";
> > > + ctx->supplies[1].supply = "vddio";
> > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> > > + ctx->supplies);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to get regulators: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR(ctx->reset_gpio)) {
> > > + ret = PTR_ERR(ctx->reset_gpio);
> > > + dev_err(dev, "Failed to get reset-gpios: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ctx->dsi = dsi;
> > > + mipi_dsi_set_drvdata(dsi, ctx);
> > > +
> > > + dsi->lanes = 4;
> > > + dsi->format = MIPI_DSI_FMT_RGB888;
> > > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > > + MIPI_DSI_MODE_VIDEO_HSE |
> > MIPI_DSI_MODE_EOT_PACKET |
> > > + MIPI_DSI_CLOCK_NON_CONTINUOUS |
> > MIPI_DSI_MODE_LPM;
> > > +
> > > + drm_panel_init(&ctx->panel, dev, &tm5p5_nt35596_panel_funcs,
> > > + DRM_MODE_CONNECTOR_DSI);
> > > +
> > > + ctx->panel.backlight = tm5p5_nt35596_create_backlight(dsi);
> > > + if (IS_ERR(ctx->panel.backlight)) {
> > > + ret = PTR_ERR(ctx->panel.backlight);
> > > + dev_err(dev, "Failed to create backlight: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = drm_panel_add(&ctx->panel);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to add panel: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = mipi_dsi_attach(dsi);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to attach to DSI host: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tm5p5_nt35596_remove(struct mipi_dsi_device *dsi)
> > > +{
> > > + struct tm5p5_nt35596 *ctx = mipi_dsi_get_drvdata(dsi);
> > > + int ret;
> > > +
> > > + ret = mipi_dsi_detach(dsi);
> > > + if (ret < 0)
> > > + dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n",
> > ret);
> > Line too long I think
> >
> > > +
> > > + drm_panel_remove(&ctx->panel);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id tm5p5_nt35596_of_match[] = {
> > > + { .compatible = "tm5p5,nt35596" },
> > vendor tm5p5 is not documented.
> >
> > > + { /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, tm5p5_nt35596_of_match);
> > > +
> > > +static struct mipi_dsi_driver tm5p5_nt35596_driver = {
> > > + .probe = tm5p5_nt35596_probe,
> > > + .remove = tm5p5_nt35596_remove,
> > > + .driver = {
> > > + .name = "panel-tm5p5-nt35596",
> > > + .of_match_table = tm5p5_nt35596_of_match,
> > > + },
> > > +};
> > > +module_mipi_dsi_driver(tm5p5_nt35596_driver);
> > > +
> > > +MODULE_AUTHOR("Konrad Dybcio <[email protected]>");
> > > +MODULE_DESCRIPTION("DRM driver for tm5p5 nt35596 1080p video mode dsi
> > panel");
> > > +MODULE_LICENSE("GPL v2");
> > > --
> > > 2.26.1
> >