[PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[Maxime Ripard]

Hi,

Here is an attempt at supporting the ST7789V LCD controller from Sitronix.

It is controlled through an SPI bus, with a twist, since each byte sent
must be prefixed by a bit, which needs an 9-bits-per-word SPI controller,
which is quite rare. Else, you would need to bitbang it.

Let me know what you think,
Maxime

Maxime Ripard (2):
  dt-bindings: display: panel: Add bindings for the Sitronix ST7789V panel
  drm/panel: Add driver for sitronix ST7789V panel

 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt |  36 ++++++-
 drivers/gpu/drm/panel/Kconfig                                        |   4 +-
 drivers/gpu/drm/panel/Makefile                                       |   1 +-
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c                       | 435 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 476 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
 create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c

base-commit: 2a6e628efb37432a83bb42b00e3c403b1d8873dd
-- 
git-series 0.8.11

[PATCH 1/2] dt-bindings: display: panel: Add bindings for the Sitronix ST7789V panel

[Maxime Ripard]

The Sitronix ST7789V is an LCD panel controller, controlled over SPI, that
can drive 18-bits 240x320 LCD displays.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt

diff --git a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
new file mode 100644
index 000000000000..f70e6e9c54ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
@@ -0,0 +1,36 @@
+Sitronix ST7789V RGB panel with SPI control bus
+
+Required properties:
+  - compatible: "sitronix,st7789v"
+  - reg: Chip select of the panel on the SPI bus
+  - reset-gpios: a GPIO phandle for the reset pin
+
+Optional properties:
+  - backlight: phandle to the backlight used
+
+The generic bindings for the SPI slaves documented in [1] also applies
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in [2]. This
+node should describe panel's video bus.
+
+[1]: Documentation/devicetree/bindings/spi/spi-bus.txt
+[2]: Documentation/devicetree/bindings/graph.txt
+
+Example:
+
+panel@0 {
+	compatible = "sitronix,st7789v";
+	reg = <0>;
+	reset-gpios = <&pio 6 11 GPIO_ACTIVE_LOW>;
+	backlight = <&pwm_bl>;
+	spi-max-frequency = <100000>;
+	spi-cpol;
+	spi-cpha;
+
+	port {
+		panel_input: endpoint {
+			remote-endpoint = <&tcon0_out_panel>;
+		};
+	};
+};
-- 
git-series 0.8.11

Re: [PATCH 1/2] dt-bindings: display: panel: Add bindings for the Sitronix ST7789V panel

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

On Fri, Feb 03, 2017 at 10:59:05AM +0100, Maxime Ripard wrote:
> The Sitronix ST7789V is an LCD panel controller, controlled over SPI, that
> can drive 18-bits 240x320 LCD displays.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> new file mode 100644
> index 000000000000..f70e6e9c54ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> @@ -0,0 +1,36 @@
> +Sitronix ST7789V RGB panel with SPI control bus
> +
> +Required properties:
> +  - compatible: "sitronix,st7789v"
> +  - reg: Chip select of the panel on the SPI bus
> +  - reset-gpios: a GPIO phandle for the reset pin

Shouldn't this have some sort of power supply?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] dt-bindings: display: panel: Add bindings for the Sitronix ST7789V panel

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

Hi Thierry,

On Mon, Feb 06, 2017 at 02:05:49PM +0100, Thierry Reding wrote:
> On Fri, Feb 03, 2017 at 10:59:05AM +0100, Maxime Ripard wrote:
> > The Sitronix ST7789V is an LCD panel controller, controlled over SPI, that
> > can drive 18-bits 240x320 LCD displays.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> > new file mode 100644
> > index 000000000000..f70e6e9c54ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> > @@ -0,0 +1,36 @@
> > +Sitronix ST7789V RGB panel with SPI control bus
> > +
> > +Required properties:
> > +  - compatible: "sitronix,st7789v"
> > +  - reg: Chip select of the panel on the SPI bus
> > +  - reset-gpios: a GPIO phandle for the reset pin
> 
> Shouldn't this have some sort of power supply?

On the board I currently have, it's just tied to an always on
regulator, but you're right, I'll add it.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/2] dt-bindings: display: panel: Add bindings for the Sitronix ST7789V panel

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On Tue, Feb 07, 2017 at 04:44:42PM +0100, Maxime Ripard wrote:
> Hi Thierry,
> 
> On Mon, Feb 06, 2017 at 02:05:49PM +0100, Thierry Reding wrote:
> > On Fri, Feb 03, 2017 at 10:59:05AM +0100, Maxime Ripard wrote:
> > > The Sitronix ST7789V is an LCD panel controller, controlled over SPI, that
> > > can drive 18-bits 240x320 LCD displays.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > >  Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt | 36 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 36 insertions(+), 0 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> > > new file mode 100644
> > > index 000000000000..f70e6e9c54ed
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
> > > @@ -0,0 +1,36 @@
> > > +Sitronix ST7789V RGB panel with SPI control bus
> > > +
> > > +Required properties:
> > > +  - compatible: "sitronix,st7789v"
> > > +  - reg: Chip select of the panel on the SPI bus
> > > +  - reset-gpios: a GPIO phandle for the reset pin
> > 
> > Shouldn't this have some sort of power supply?
> 
> On the board I currently have, it's just tied to an always on
> regulator, but you're right, I'll add it.

Yes, we need to make sure to describe this from the panel's perspective
to make sure that when other board designers hook things up differently
we can still model it in DT.

But you already knew that...

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/panel/Kconfig                  |   4 +-
 drivers/gpu/drm/panel/Makefile                 |   1 +-
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
 3 files changed, 440 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 62aba976e744..d07b996a07b8 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
 	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
 	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
 
+config DRM_PANEL_SITRONIX_ST7789V
+	tristate "Sitronx ST7789V panel"
+	depends on OF && SPI
+
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index a5c7ec0236e0..41b245d39984 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
+obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
new file mode 100644
index 000000000000..aab817b55aa6
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -0,0 +1,435 @@
+/*
+ * Copyright (C) 2017 Free Electrons
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_panel.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/spi/spi.h>
+
+#define ST7789V_SLPIN_CMD		0x10
+
+#define ST7789V_SLPOUT_CMD		0x11
+
+#define ST7789V_INVON_CMD		0x21
+
+#define ST7789V_DISPOFF_CMD		0x28
+
+#define ST7789V_DISPON_CMD		0x29
+
+#define ST7789V_MADCTL_CMD		0x36
+
+#define ST7789V_COLMOD_CMD		0x3a
+#define ST7789V_COLMOD_RGB_FMT_18BITS		(6 << 4)
+#define ST7789V_COLMOD_CTRL_FMT_18BITS		(6 << 0)
+
+#define ST7789V_RAMCTRL_CMD		0xb0
+#define ST7789V_RAMCTRL_RM_RGB			BIT(4)
+#define ST7789V_RAMCTRL_DM_RGB			BIT(0)
+#define ST7789V_RAMCTRL_MAGIC			(3 << 6)
+#define ST7789V_RAMCTRL_EPF(n)			(((n) & 3) << 4)
+
+#define ST7789V_RGBCTRL_CMD		0xb1
+#define ST7789V_RGBCTRL_WO			BIT(7)
+#define ST7789V_RGBCTRL_RCM(n)			(((n) & 3) << 5)
+#define ST7789V_RGBCTRL_VSYNC_HIGH		BIT(3)
+#define ST7789V_RGBCTRL_HSYNC_HIGH		BIT(2)
+#define ST7789V_RGBCTRL_PCLK_HIGH		BIT(1)
+#define ST7789V_RGBCTRL_VBP(n)			((n) & 0x7f)
+#define ST7789V_RGBCTRL_HBP(n)			((n) & 0x1f)
+
+#define ST7789V_PORCTRL_CMD		0xb2
+#define ST7789V_PORCTRL_IDLE_BP(n)		(((n) & 0xf) << 4)
+#define ST7789V_PORCTRL_IDLE_FP(n)		((n) & 0xf)
+#define ST7789V_PORCTRL_PARTIAL_BP(n)		(((n) & 0xf) << 4)
+#define ST7789V_PORCTRL_PARTIAL_FP(n)		((n) & 0xf)
+
+#define ST7789V_GCTRL_CMD		0xb7
+#define ST7789V_GCTRL_VGHS(n)			(((n) & 7) << 4)
+#define ST7789V_GCTRL_VGLS(n)			((n) & 7)
+
+#define ST7789V_VCOMS_CMD		0xbb
+
+#define ST7789V_LCMCTRL_CMD		0xc0
+#define ST7789V_LCMCTRL_XBGR			BIT(5)
+#define ST7789V_LCMCTRL_XMX			BIT(3)
+#define ST7789V_LCMCTRL_XMH			BIT(2)
+
+#define ST7789V_VDVVRHEN_CMD		0xc2
+#define ST7789V_VDVVRHEN_CMDEN			BIT(0)
+
+#define ST7789V_VRHS_CMD		0xc3
+
+#define ST7789V_VDVS_CMD		0xc4
+
+#define ST7789V_FRCTRL2_CMD		0xc6
+
+#define ST7789V_PWCTRL1_CMD		0xd0
+#define ST7789V_PWCTRL1_MAGIC			0xa4
+#define ST7789V_PWCTRL1_AVDD(n)			(((n) & 3) << 6)
+#define ST7789V_PWCTRL1_AVCL(n)			(((n) & 3) << 4)
+#define ST7789V_PWCTRL1_VDS(n)			((n) & 3)
+
+#define ST7789V_PVGAMCTRL_CMD		0xe0
+#define ST7789V_PVGAMCTRL_JP0(n)		(((n) & 3) << 4)
+#define ST7789V_PVGAMCTRL_JP1(n)		(((n) & 3) << 4)
+#define ST7789V_PVGAMCTRL_VP0(n)		((n) & 0xf)
+#define ST7789V_PVGAMCTRL_VP1(n)		((n) & 0x3f)
+#define ST7789V_PVGAMCTRL_VP2(n)		((n) & 0x3f)
+#define ST7789V_PVGAMCTRL_VP4(n)		((n) & 0x1f)
+#define ST7789V_PVGAMCTRL_VP6(n)		((n) & 0x1f)
+#define ST7789V_PVGAMCTRL_VP13(n)		((n) & 0xf)
+#define ST7789V_PVGAMCTRL_VP20(n)		((n) & 0x7f)
+#define ST7789V_PVGAMCTRL_VP27(n)		((n) & 7)
+#define ST7789V_PVGAMCTRL_VP36(n)		(((n) & 7) << 4)
+#define ST7789V_PVGAMCTRL_VP43(n)		((n) & 0x7f)
+#define ST7789V_PVGAMCTRL_VP50(n)		((n) & 0xf)
+#define ST7789V_PVGAMCTRL_VP57(n)		((n) & 0x1f)
+#define ST7789V_PVGAMCTRL_VP59(n)		((n) & 0x1f)
+#define ST7789V_PVGAMCTRL_VP61(n)		((n) & 0x3f)
+#define ST7789V_PVGAMCTRL_VP62(n)		((n) & 0x3f)
+#define ST7789V_PVGAMCTRL_VP63(n)		(((n) & 0xf) << 4)
+
+#define ST7789V_NVGAMCTRL_CMD		0xe1
+#define ST7789V_NVGAMCTRL_JN0(n)		(((n) & 3) << 4)
+#define ST7789V_NVGAMCTRL_JN1(n)		(((n) & 3) << 4)
+#define ST7789V_NVGAMCTRL_VN0(n)		((n) & 0xf)
+#define ST7789V_NVGAMCTRL_VN1(n)		((n) & 0x3f)
+#define ST7789V_NVGAMCTRL_VN2(n)		((n) & 0x3f)
+#define ST7789V_NVGAMCTRL_VN4(n)		((n) & 0x1f)
+#define ST7789V_NVGAMCTRL_VN6(n)		((n) & 0x1f)
+#define ST7789V_NVGAMCTRL_VN13(n)		((n) & 0xf)
+#define ST7789V_NVGAMCTRL_VN20(n)		((n) & 0x7f)
+#define ST7789V_NVGAMCTRL_VN27(n)		((n) & 7)
+#define ST7789V_NVGAMCTRL_VN36(n)		(((n) & 7) << 4)
+#define ST7789V_NVGAMCTRL_VN43(n)		((n) & 0x7f)
+#define ST7789V_NVGAMCTRL_VN50(n)		((n) & 0xf)
+#define ST7789V_NVGAMCTRL_VN57(n)		((n) & 0x1f)
+#define ST7789V_NVGAMCTRL_VN59(n)		((n) & 0x1f)
+#define ST7789V_NVGAMCTRL_VN61(n)		((n) & 0x3f)
+#define ST7789V_NVGAMCTRL_VN62(n)		((n) & 0x3f)
+#define ST7789V_NVGAMCTRL_VN63(n)		(((n) & 0xf) << 4)
+
+struct st7789v {
+	struct drm_panel panel;
+	struct spi_device *spi;
+	struct gpio_desc *reset;
+	struct backlight_device *backlight;
+};
+
+enum st7789v_prefix {
+	ST7789V_COMMAND = 0,
+	ST7789V_DATA = 1,
+};
+
+static inline struct st7789v *panel_to_st7789v(struct drm_panel *panel)
+{
+	return container_of(panel, struct st7789v, panel);
+}
+
+static int st7789v_spi_write(struct st7789v *ctx, u8 prefix, u8 data)
+{
+	struct spi_transfer xfer = { };
+	struct spi_message msg;
+	u16 txbuf = ((prefix & 1) << 8) | data;
+
+	spi_message_init(&msg);
+
+	xfer.tx_buf = &txbuf;
+	xfer.bits_per_word = 9;
+	xfer.len = sizeof(txbuf);
+
+	spi_message_add_tail(&xfer, &msg);
+	return spi_sync(ctx->spi, &msg);
+}
+
+static int st7789v_write_command(struct st7789v *ctx, u8 cmd)
+{
+	return st7789v_spi_write(ctx, ST7789V_COMMAND, cmd);
+}
+
+static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
+{
+	return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
+}
+
+static int st7789v_write_command_data(struct st7789v *ctx, u8 cmd,
+				      unsigned long n_data, ...)
+{
+	va_list ap;
+	int i;
+
+	st7789v_write_command(ctx, cmd);
+
+	va_start(ap, n_data);
+	for (i = 0; i < n_data; i++)
+		st7789v_write_data(ctx, va_arg(ap, int));
+	va_end(ap);
+
+	return 0;
+}
+
+static const struct drm_display_mode default_mode = {
+	.clock = 7000,
+	.hdisplay = 240,
+	.hsync_start = 240 + 38,
+	.hsync_end = 240 + 38 + 10,
+	.htotal = 240 + 38 + 10 + 10,
+	.vdisplay = 320,
+	.vsync_start = 320 + 8,
+	.vsync_end = 320 + 8 + 4,
+	.vtotal = 320 + 8 + 4 + 4,
+	.vrefresh = 60,
+};
+
+static int st7789v_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &default_mode);
+	if (!mode) {
+		dev_err(panel->drm->dev, "failed to add mode %ux%ux@%u\n",
+			default_mode.hdisplay, default_mode.vdisplay,
+			default_mode.vrefresh);
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	panel->connector->display_info.width_mm = 61;
+	panel->connector->display_info.height_mm = 103;
+
+	return 1;
+}
+
+#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
+#define st7789v_send(ctx, cmd, ...)					\
+	st7789v_write_command_data(ctx, cmd, NUMARGS(__VA_ARGS__),	\
+				   ##__VA_ARGS__)
+
+static int st7789v_prepare(struct drm_panel *panel)
+{
+	struct st7789v *ctx = panel_to_st7789v(panel);
+
+	gpiod_set_value(ctx->reset, 1);
+	msleep(30);
+	gpiod_set_value(ctx->reset, 0);
+	msleep(120);
+
+	st7789v_send(ctx, ST7789V_SLPOUT_CMD);
+
+	/* We need to wait 120ms after a sleep out command */
+	msleep(120);
+
+	st7789v_send(ctx, ST7789V_MADCTL_CMD, 0);
+
+	st7789v_send(ctx, ST7789V_COLMOD_CMD,
+		     ST7789V_COLMOD_CTRL_FMT_18BITS |
+		     ST7789V_COLMOD_RGB_FMT_18BITS);
+
+	st7789v_send(ctx, ST7789V_PORCTRL_CMD,
+		     0xc,		/* Backporch, normal mode*/
+		     0xc,		/* Frontporch, normal mode */
+		     0,			/* Disable separate porch control */
+		     ST7789V_PORCTRL_IDLE_BP(3) |
+		     ST7789V_PORCTRL_IDLE_FP(3),
+		     ST7789V_PORCTRL_PARTIAL_BP(3) |
+		     ST7789V_PORCTRL_PARTIAL_FP(3));
+
+	st7789v_send(ctx, ST7789V_GCTRL_CMD,
+		     ST7789V_GCTRL_VGLS(5) | ST7789V_GCTRL_VGHS(3));
+
+	st7789v_send(ctx, ST7789V_VCOMS_CMD, 0x2b);
+
+	st7789v_send(ctx, ST7789V_LCMCTRL_CMD, ST7789V_LCMCTRL_XMH |
+		     ST7789V_LCMCTRL_XMX | ST7789V_LCMCTRL_XBGR);
+
+	st7789v_send(ctx, ST7789V_VDVVRHEN_CMD, ST7789V_VDVVRHEN_CMDEN);
+
+	st7789v_send(ctx, ST7789V_VRHS_CMD, 0xf);
+
+	st7789v_send(ctx, ST7789V_VDVS_CMD, 0x20);
+
+	st7789v_send(ctx, ST7789V_FRCTRL2_CMD, 0xf);
+
+	st7789v_send(ctx, ST7789V_PWCTRL1_CMD, ST7789V_PWCTRL1_MAGIC,
+		     ST7789V_PWCTRL1_AVDD(2) | ST7789V_PWCTRL1_AVCL(2) |
+		     ST7789V_PWCTRL1_VDS(1));
+
+	st7789v_send(ctx, ST7789V_PVGAMCTRL_CMD,
+		     ST7789V_PVGAMCTRL_VP63(0xd),
+		     ST7789V_PVGAMCTRL_VP1(0xca),
+		     ST7789V_PVGAMCTRL_VP2(0xe),
+		     ST7789V_PVGAMCTRL_VP4(8),
+		     ST7789V_PVGAMCTRL_VP6(9),
+		     ST7789V_PVGAMCTRL_VP13(7),
+		     ST7789V_PVGAMCTRL_VP20(0x2d),
+		     ST7789V_PVGAMCTRL_VP27(0xb) | ST7789V_PVGAMCTRL_VP36(3),
+		     ST7789V_PVGAMCTRL_VP43(0x3d),
+		     ST7789V_PVGAMCTRL_JP1(3) | ST7789V_PVGAMCTRL_VP50(4),
+		     ST7789V_PVGAMCTRL_VP57(0xa),
+		     ST7789V_PVGAMCTRL_VP59(0xa),
+		     ST7789V_PVGAMCTRL_VP61(0x1b),
+		     ST7789V_PVGAMCTRL_VP62(0x28));
+
+	st7789v_send(ctx, ST7789V_NVGAMCTRL_CMD,
+		     ST7789V_NVGAMCTRL_VN63(0xd),
+		     ST7789V_NVGAMCTRL_VN1(0xca),
+		     ST7789V_NVGAMCTRL_VN2(0xf),
+		     ST7789V_NVGAMCTRL_VN4(8),
+		     ST7789V_NVGAMCTRL_VN6(8),
+		     ST7789V_NVGAMCTRL_VN13(7),
+		     ST7789V_NVGAMCTRL_VN20(0x2e),
+		     ST7789V_NVGAMCTRL_VN27(0xc) | ST7789V_NVGAMCTRL_VN36(5),
+		     ST7789V_NVGAMCTRL_VN43(0x40),
+		     ST7789V_NVGAMCTRL_JN1(3) | ST7789V_NVGAMCTRL_VN50(4),
+		     ST7789V_NVGAMCTRL_VN57(9),
+		     ST7789V_NVGAMCTRL_VN59(0xb),
+		     ST7789V_NVGAMCTRL_VN61(0x1b),
+		     ST7789V_NVGAMCTRL_VN62(0x28));
+
+	st7789v_send(ctx, ST7789V_INVON_CMD);
+
+	st7789v_send(ctx, ST7789V_RAMCTRL_CMD,
+		     ST7789V_RAMCTRL_DM_RGB | ST7789V_RAMCTRL_RM_RGB,
+		     ST7789V_RAMCTRL_EPF(3) | ST7789V_RAMCTRL_MAGIC);
+
+	st7789v_send(ctx, ST7789V_RGBCTRL_CMD,
+		     ST7789V_RGBCTRL_WO | ST7789V_RGBCTRL_RCM(2) |
+		     ST7789V_RGBCTRL_VSYNC_HIGH | ST7789V_RGBCTRL_HSYNC_HIGH |
+		     ST7789V_RGBCTRL_PCLK_HIGH,
+		     ST7789V_RGBCTRL_VBP(8), ST7789V_RGBCTRL_HBP(20));
+
+	return 0;
+}
+
+static int st7789v_enable(struct drm_panel *panel)
+{
+	struct st7789v *ctx = panel_to_st7789v(panel);
+
+	if (ctx->backlight) {
+		ctx->backlight->props.state &= ~BL_CORE_FBBLANK;
+		ctx->backlight->props.power = FB_BLANK_UNBLANK;
+		backlight_update_status(ctx->backlight);
+	}
+
+	return st7789v_send(ctx, ST7789V_DISPON_CMD);
+}
+
+static int st7789v_disable(struct drm_panel *panel)
+{
+	struct st7789v *ctx = panel_to_st7789v(panel);
+
+	st7789v_send(ctx, ST7789V_DISPOFF_CMD);
+
+	if (ctx->backlight) {
+		ctx->backlight->props.power = FB_BLANK_POWERDOWN;
+		ctx->backlight->props.state |= BL_CORE_FBBLANK;
+		backlight_update_status(ctx->backlight);
+	}
+
+	return 0;
+}
+
+static int st7789v_unprepare(struct drm_panel *panel)
+{
+	struct st7789v *ctx = panel_to_st7789v(panel);
+
+	st7789v_send(ctx, ST7789V_SLPIN_CMD);
+
+	return 0;
+}
+
+static const struct drm_panel_funcs st7789v_drm_funcs = {
+	.disable	= st7789v_disable,
+	.enable		= st7789v_enable,
+	.get_modes	= st7789v_get_modes,
+	.prepare	= st7789v_prepare,
+	.unprepare	= st7789v_unprepare,
+};
+
+static int st7789v_probe(struct spi_device *spi)
+{
+	struct device_node *backlight;
+	struct st7789v *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(&spi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, ctx);
+	ctx->spi = spi;
+
+	ctx->panel.dev = &spi->dev;
+	ctx->panel.funcs = &st7789v_drm_funcs;
+
+	ctx->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset)) {
+		dev_err(&spi->dev, "Couldn't get our reset line\n");
+		return PTR_ERR(ctx->reset);
+	}
+
+	backlight = of_parse_phandle(spi->dev.of_node, "backlight", 0);
+	if (backlight) {
+		ctx->backlight = of_find_backlight_by_node(backlight);
+		of_node_put(backlight);
+
+		if (!ctx->backlight)
+			return -EPROBE_DEFER;
+	}
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret < 0)
+		goto err_free_backlight;
+
+	return 0;
+
+err_free_backlight:
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	return ret;
+}
+
+static int st7789v_remove(struct spi_device *spi)
+{
+	struct st7789v *ctx = spi_get_drvdata(spi);
+
+	drm_panel_detach(&ctx->panel);
+	drm_panel_remove(&ctx->panel);
+
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	return 0;
+}
+
+static const struct of_device_id st7789v_of_match[] = {
+	{ .compatible = "sitronix,st7789v" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, st7789v_of_match);
+
+static struct spi_driver st7789v_driver = {
+	.probe = st7789v_probe,
+	.remove = st7789v_remove,
+	.driver = {
+		.name = "st7789v",
+		.of_match_table = st7789v_of_match,
+	},
+};
+module_spi_driver(st7789v_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_DESCRIPTION("Sitronix st7789v LCD Driver");
+MODULE_LICENSE("GPL v2");
-- 
git-series 0.8.11

Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Noralf Trønnes]

Den 03.02.2017 10.59, skrev Maxime Ripard:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>   drivers/gpu/drm/panel/Kconfig                  |   4 +-
>   drivers/gpu/drm/panel/Makefile                 |   1 +-
>   drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
>   3 files changed, 440 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 62aba976e744..d07b996a07b8 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
>   	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
>   	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
>   
> +config DRM_PANEL_SITRONIX_ST7789V
> +	tristate "Sitronx ST7789V panel"
> +	depends on OF && SPI
> +
>   endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index a5c7ec0236e0..41b245d39984 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>   obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>   obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>   obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> +obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> new file mode 100644
> index 000000000000..aab817b55aa6
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -0,0 +1,435 @@
> +/*
> + * Copyright (C) 2017 Free Electrons
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#define ST7789V_SLPIN_CMD		0x10
> +
> +#define ST7789V_SLPOUT_CMD		0x11
> +
> +#define ST7789V_INVON_CMD		0x21
> +
> +#define ST7789V_DISPOFF_CMD		0x28
> +
> +#define ST7789V_DISPON_CMD		0x29
> +
> +#define ST7789V_MADCTL_CMD		0x36
> +
> +#define ST7789V_COLMOD_CMD		0x3a

You can use these from include/video/mipi_display.h:

         MIPI_DCS_ENTER_SLEEP_MODE       = 0x10,
         MIPI_DCS_EXIT_SLEEP_MODE        = 0x11,
         MIPI_DCS_ENTER_INVERT_MODE      = 0x21,
         MIPI_DCS_SET_DISPLAY_OFF        = 0x28,
         MIPI_DCS_SET_DISPLAY_ON         = 0x29,
         MIPI_DCS_SET_ADDRESS_MODE       = 0x36,
         MIPI_DCS_SET_PIXEL_FORMAT       = 0x3A,


Noralf.

> +#define ST7789V_COLMOD_RGB_FMT_18BITS		(6 << 4)
> +#define ST7789V_COLMOD_CTRL_FMT_18BITS		(6 << 0)
> +
> +#define ST7789V_RAMCTRL_CMD		0xb0
> +#define ST7789V_RAMCTRL_RM_RGB			BIT(4)
> +#define ST7789V_RAMCTRL_DM_RGB			BIT(0)
> +#define ST7789V_RAMCTRL_MAGIC			(3 << 6)
> +#define ST7789V_RAMCTRL_EPF(n)			(((n) & 3) << 4)
> +
> +#define ST7789V_RGBCTRL_CMD		0xb1
> +#define ST7789V_RGBCTRL_WO			BIT(7)
> +#define ST7789V_RGBCTRL_RCM(n)			(((n) & 3) << 5)
> +#define ST7789V_RGBCTRL_VSYNC_HIGH		BIT(3)
> +#define ST7789V_RGBCTRL_HSYNC_HIGH		BIT(2)
> +#define ST7789V_RGBCTRL_PCLK_HIGH		BIT(1)
> +#define ST7789V_RGBCTRL_VBP(n)			((n) & 0x7f)
> +#define ST7789V_RGBCTRL_HBP(n)			((n) & 0x1f)
> +
> +#define ST7789V_PORCTRL_CMD		0xb2
> +#define ST7789V_PORCTRL_IDLE_BP(n)		(((n) & 0xf) << 4)
> +#define ST7789V_PORCTRL_IDLE_FP(n)		((n) & 0xf)
> +#define ST7789V_PORCTRL_PARTIAL_BP(n)		(((n) & 0xf) << 4)
> +#define ST7789V_PORCTRL_PARTIAL_FP(n)		((n) & 0xf)
> +
> +#define ST7789V_GCTRL_CMD		0xb7
> +#define ST7789V_GCTRL_VGHS(n)			(((n) & 7) << 4)
> +#define ST7789V_GCTRL_VGLS(n)			((n) & 7)
> +
> +#define ST7789V_VCOMS_CMD		0xbb
> +
> +#define ST7789V_LCMCTRL_CMD		0xc0
> +#define ST7789V_LCMCTRL_XBGR			BIT(5)
> +#define ST7789V_LCMCTRL_XMX			BIT(3)
> +#define ST7789V_LCMCTRL_XMH			BIT(2)
> +
> +#define ST7789V_VDVVRHEN_CMD		0xc2
> +#define ST7789V_VDVVRHEN_CMDEN			BIT(0)
> +
> +#define ST7789V_VRHS_CMD		0xc3
> +
> +#define ST7789V_VDVS_CMD		0xc4
> +
> +#define ST7789V_FRCTRL2_CMD		0xc6
> +
> +#define ST7789V_PWCTRL1_CMD		0xd0
> +#define ST7789V_PWCTRL1_MAGIC			0xa4
> +#define ST7789V_PWCTRL1_AVDD(n)			(((n) & 3) << 6)
> +#define ST7789V_PWCTRL1_AVCL(n)			(((n) & 3) << 4)
> +#define ST7789V_PWCTRL1_VDS(n)			((n) & 3)
> +
> +#define ST7789V_PVGAMCTRL_CMD		0xe0
> +#define ST7789V_PVGAMCTRL_JP0(n)		(((n) & 3) << 4)
> +#define ST7789V_PVGAMCTRL_JP1(n)		(((n) & 3) << 4)
> +#define ST7789V_PVGAMCTRL_VP0(n)		((n) & 0xf)
> +#define ST7789V_PVGAMCTRL_VP1(n)		((n) & 0x3f)
> +#define ST7789V_PVGAMCTRL_VP2(n)		((n) & 0x3f)
> +#define ST7789V_PVGAMCTRL_VP4(n)		((n) & 0x1f)
> +#define ST7789V_PVGAMCTRL_VP6(n)		((n) & 0x1f)
> +#define ST7789V_PVGAMCTRL_VP13(n)		((n) & 0xf)
> +#define ST7789V_PVGAMCTRL_VP20(n)		((n) & 0x7f)
> +#define ST7789V_PVGAMCTRL_VP27(n)		((n) & 7)
> +#define ST7789V_PVGAMCTRL_VP36(n)		(((n) & 7) << 4)
> +#define ST7789V_PVGAMCTRL_VP43(n)		((n) & 0x7f)
> +#define ST7789V_PVGAMCTRL_VP50(n)		((n) & 0xf)
> +#define ST7789V_PVGAMCTRL_VP57(n)		((n) & 0x1f)
> +#define ST7789V_PVGAMCTRL_VP59(n)		((n) & 0x1f)
> +#define ST7789V_PVGAMCTRL_VP61(n)		((n) & 0x3f)
> +#define ST7789V_PVGAMCTRL_VP62(n)		((n) & 0x3f)
> +#define ST7789V_PVGAMCTRL_VP63(n)		(((n) & 0xf) << 4)
> +
> +#define ST7789V_NVGAMCTRL_CMD		0xe1
> +#define ST7789V_NVGAMCTRL_JN0(n)		(((n) & 3) << 4)
> +#define ST7789V_NVGAMCTRL_JN1(n)		(((n) & 3) << 4)
> +#define ST7789V_NVGAMCTRL_VN0(n)		((n) & 0xf)
> +#define ST7789V_NVGAMCTRL_VN1(n)		((n) & 0x3f)
> +#define ST7789V_NVGAMCTRL_VN2(n)		((n) & 0x3f)
> +#define ST7789V_NVGAMCTRL_VN4(n)		((n) & 0x1f)
> +#define ST7789V_NVGAMCTRL_VN6(n)		((n) & 0x1f)
> +#define ST7789V_NVGAMCTRL_VN13(n)		((n) & 0xf)
> +#define ST7789V_NVGAMCTRL_VN20(n)		((n) & 0x7f)
> +#define ST7789V_NVGAMCTRL_VN27(n)		((n) & 7)
> +#define ST7789V_NVGAMCTRL_VN36(n)		(((n) & 7) << 4)
> +#define ST7789V_NVGAMCTRL_VN43(n)		((n) & 0x7f)
> +#define ST7789V_NVGAMCTRL_VN50(n)		((n) & 0xf)
> +#define ST7789V_NVGAMCTRL_VN57(n)		((n) & 0x1f)
> +#define ST7789V_NVGAMCTRL_VN59(n)		((n) & 0x1f)
> +#define ST7789V_NVGAMCTRL_VN61(n)		((n) & 0x3f)
> +#define ST7789V_NVGAMCTRL_VN62(n)		((n) & 0x3f)
> +#define ST7789V_NVGAMCTRL_VN63(n)		(((n) & 0xf) << 4)
> +
> +struct st7789v {
> +	struct drm_panel panel;
> +	struct spi_device *spi;
> +	struct gpio_desc *reset;
> +	struct backlight_device *backlight;
> +};
> +
> +enum st7789v_prefix {
> +	ST7789V_COMMAND = 0,
> +	ST7789V_DATA = 1,
> +};
> +
> +static inline struct st7789v *panel_to_st7789v(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct st7789v, panel);
> +}
> +
> +static int st7789v_spi_write(struct st7789v *ctx, u8 prefix, u8 data)
> +{
> +	struct spi_transfer xfer = { };
> +	struct spi_message msg;
> +	u16 txbuf = ((prefix & 1) << 8) | data;
> +
> +	spi_message_init(&msg);
> +
> +	xfer.tx_buf = &txbuf;
> +	xfer.bits_per_word = 9;
> +	xfer.len = sizeof(txbuf);
> +
> +	spi_message_add_tail(&xfer, &msg);
> +	return spi_sync(ctx->spi, &msg);
> +}
> +
> +static int st7789v_write_command(struct st7789v *ctx, u8 cmd)
> +{
> +	return st7789v_spi_write(ctx, ST7789V_COMMAND, cmd);
> +}
> +
> +static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
> +{
> +	return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
> +}
> +
> +static int st7789v_write_command_data(struct st7789v *ctx, u8 cmd,
> +				      unsigned long n_data, ...)
> +{
> +	va_list ap;
> +	int i;
> +
> +	st7789v_write_command(ctx, cmd);
> +
> +	va_start(ap, n_data);
> +	for (i = 0; i < n_data; i++)
> +		st7789v_write_data(ctx, va_arg(ap, int));
> +	va_end(ap);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> +	.clock = 7000,
> +	.hdisplay = 240,
> +	.hsync_start = 240 + 38,
> +	.hsync_end = 240 + 38 + 10,
> +	.htotal = 240 + 38 + 10 + 10,
> +	.vdisplay = 320,
> +	.vsync_start = 320 + 8,
> +	.vsync_end = 320 + 8 + 4,
> +	.vtotal = 320 + 8 + 4 + 4,
> +	.vrefresh = 60,
> +};
> +
> +static int st7789v_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &default_mode);
> +	if (!mode) {
> +		dev_err(panel->drm->dev, "failed to add mode %ux%ux@%u\n",
> +			default_mode.hdisplay, default_mode.vdisplay,
> +			default_mode.vrefresh);
> +		return -ENOMEM;
> +	}
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	panel->connector->display_info.width_mm = 61;
> +	panel->connector->display_info.height_mm = 103;
> +
> +	return 1;
> +}
> +
> +#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
> +#define st7789v_send(ctx, cmd, ...)					\
> +	st7789v_write_command_data(ctx, cmd, NUMARGS(__VA_ARGS__),	\
> +				   ##__VA_ARGS__)
> +
> +static int st7789v_prepare(struct drm_panel *panel)
> +{
> +	struct st7789v *ctx = panel_to_st7789v(panel);
> +
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(30);
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(120);
> +
> +	st7789v_send(ctx, ST7789V_SLPOUT_CMD);
> +
> +	/* We need to wait 120ms after a sleep out command */
> +	msleep(120);
> +
> +	st7789v_send(ctx, ST7789V_MADCTL_CMD, 0);
> +
> +	st7789v_send(ctx, ST7789V_COLMOD_CMD,
> +		     ST7789V_COLMOD_CTRL_FMT_18BITS |
> +		     ST7789V_COLMOD_RGB_FMT_18BITS);
> +
> +	st7789v_send(ctx, ST7789V_PORCTRL_CMD,
> +		     0xc,		/* Backporch, normal mode*/
> +		     0xc,		/* Frontporch, normal mode */
> +		     0,			/* Disable separate porch control */
> +		     ST7789V_PORCTRL_IDLE_BP(3) |
> +		     ST7789V_PORCTRL_IDLE_FP(3),
> +		     ST7789V_PORCTRL_PARTIAL_BP(3) |
> +		     ST7789V_PORCTRL_PARTIAL_FP(3));
> +
> +	st7789v_send(ctx, ST7789V_GCTRL_CMD,
> +		     ST7789V_GCTRL_VGLS(5) | ST7789V_GCTRL_VGHS(3));
> +
> +	st7789v_send(ctx, ST7789V_VCOMS_CMD, 0x2b);
> +
> +	st7789v_send(ctx, ST7789V_LCMCTRL_CMD, ST7789V_LCMCTRL_XMH |
> +		     ST7789V_LCMCTRL_XMX | ST7789V_LCMCTRL_XBGR);
> +
> +	st7789v_send(ctx, ST7789V_VDVVRHEN_CMD, ST7789V_VDVVRHEN_CMDEN);
> +
> +	st7789v_send(ctx, ST7789V_VRHS_CMD, 0xf);
> +
> +	st7789v_send(ctx, ST7789V_VDVS_CMD, 0x20);
> +
> +	st7789v_send(ctx, ST7789V_FRCTRL2_CMD, 0xf);
> +
> +	st7789v_send(ctx, ST7789V_PWCTRL1_CMD, ST7789V_PWCTRL1_MAGIC,
> +		     ST7789V_PWCTRL1_AVDD(2) | ST7789V_PWCTRL1_AVCL(2) |
> +		     ST7789V_PWCTRL1_VDS(1));
> +
> +	st7789v_send(ctx, ST7789V_PVGAMCTRL_CMD,
> +		     ST7789V_PVGAMCTRL_VP63(0xd),
> +		     ST7789V_PVGAMCTRL_VP1(0xca),
> +		     ST7789V_PVGAMCTRL_VP2(0xe),
> +		     ST7789V_PVGAMCTRL_VP4(8),
> +		     ST7789V_PVGAMCTRL_VP6(9),
> +		     ST7789V_PVGAMCTRL_VP13(7),
> +		     ST7789V_PVGAMCTRL_VP20(0x2d),
> +		     ST7789V_PVGAMCTRL_VP27(0xb) | ST7789V_PVGAMCTRL_VP36(3),
> +		     ST7789V_PVGAMCTRL_VP43(0x3d),
> +		     ST7789V_PVGAMCTRL_JP1(3) | ST7789V_PVGAMCTRL_VP50(4),
> +		     ST7789V_PVGAMCTRL_VP57(0xa),
> +		     ST7789V_PVGAMCTRL_VP59(0xa),
> +		     ST7789V_PVGAMCTRL_VP61(0x1b),
> +		     ST7789V_PVGAMCTRL_VP62(0x28));
> +
> +	st7789v_send(ctx, ST7789V_NVGAMCTRL_CMD,
> +		     ST7789V_NVGAMCTRL_VN63(0xd),
> +		     ST7789V_NVGAMCTRL_VN1(0xca),
> +		     ST7789V_NVGAMCTRL_VN2(0xf),
> +		     ST7789V_NVGAMCTRL_VN4(8),
> +		     ST7789V_NVGAMCTRL_VN6(8),
> +		     ST7789V_NVGAMCTRL_VN13(7),
> +		     ST7789V_NVGAMCTRL_VN20(0x2e),
> +		     ST7789V_NVGAMCTRL_VN27(0xc) | ST7789V_NVGAMCTRL_VN36(5),
> +		     ST7789V_NVGAMCTRL_VN43(0x40),
> +		     ST7789V_NVGAMCTRL_JN1(3) | ST7789V_NVGAMCTRL_VN50(4),
> +		     ST7789V_NVGAMCTRL_VN57(9),
> +		     ST7789V_NVGAMCTRL_VN59(0xb),
> +		     ST7789V_NVGAMCTRL_VN61(0x1b),
> +		     ST7789V_NVGAMCTRL_VN62(0x28));
> +
> +	st7789v_send(ctx, ST7789V_INVON_CMD);
> +
> +	st7789v_send(ctx, ST7789V_RAMCTRL_CMD,
> +		     ST7789V_RAMCTRL_DM_RGB | ST7789V_RAMCTRL_RM_RGB,
> +		     ST7789V_RAMCTRL_EPF(3) | ST7789V_RAMCTRL_MAGIC);
> +
> +	st7789v_send(ctx, ST7789V_RGBCTRL_CMD,
> +		     ST7789V_RGBCTRL_WO | ST7789V_RGBCTRL_RCM(2) |
> +		     ST7789V_RGBCTRL_VSYNC_HIGH | ST7789V_RGBCTRL_HSYNC_HIGH |
> +		     ST7789V_RGBCTRL_PCLK_HIGH,
> +		     ST7789V_RGBCTRL_VBP(8), ST7789V_RGBCTRL_HBP(20));
> +
> +	return 0;
> +}
> +
> +static int st7789v_enable(struct drm_panel *panel)
> +{
> +	struct st7789v *ctx = panel_to_st7789v(panel);
> +
> +	if (ctx->backlight) {
> +		ctx->backlight->props.state &= ~BL_CORE_FBBLANK;
> +		ctx->backlight->props.power = FB_BLANK_UNBLANK;
> +		backlight_update_status(ctx->backlight);
> +	}
> +
> +	return st7789v_send(ctx, ST7789V_DISPON_CMD);
> +}
> +
> +static int st7789v_disable(struct drm_panel *panel)
> +{
> +	struct st7789v *ctx = panel_to_st7789v(panel);
> +
> +	st7789v_send(ctx, ST7789V_DISPOFF_CMD);
> +
> +	if (ctx->backlight) {
> +		ctx->backlight->props.power = FB_BLANK_POWERDOWN;
> +		ctx->backlight->props.state |= BL_CORE_FBBLANK;
> +		backlight_update_status(ctx->backlight);
> +	}
> +
> +	return 0;
> +}
> +
> +static int st7789v_unprepare(struct drm_panel *panel)
> +{
> +	struct st7789v *ctx = panel_to_st7789v(panel);
> +
> +	st7789v_send(ctx, ST7789V_SLPIN_CMD);
> +
> +	return 0;
> +}
> +
> +static const struct drm_panel_funcs st7789v_drm_funcs = {
> +	.disable	= st7789v_disable,
> +	.enable		= st7789v_enable,
> +	.get_modes	= st7789v_get_modes,
> +	.prepare	= st7789v_prepare,
> +	.unprepare	= st7789v_unprepare,
> +};
> +
> +static int st7789v_probe(struct spi_device *spi)
> +{
> +	struct device_node *backlight;
> +	struct st7789v *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(&spi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, ctx);
> +	ctx->spi = spi;
> +
> +	ctx->panel.dev = &spi->dev;
> +	ctx->panel.funcs = &st7789v_drm_funcs;
> +
> +	ctx->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset)) {
> +		dev_err(&spi->dev, "Couldn't get our reset line\n");
> +		return PTR_ERR(ctx->reset);
> +	}
> +
> +	backlight = of_parse_phandle(spi->dev.of_node, "backlight", 0);
> +	if (backlight) {
> +		ctx->backlight = of_find_backlight_by_node(backlight);
> +		of_node_put(backlight);
> +
> +		if (!ctx->backlight)
> +			return -EPROBE_DEFER;
> +	}
> +
> +	ret = drm_panel_add(&ctx->panel);
> +	if (ret < 0)
> +		goto err_free_backlight;
> +
> +	return 0;
> +
> +err_free_backlight:
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	return ret;
> +}
> +
> +static int st7789v_remove(struct spi_device *spi)
> +{
> +	struct st7789v *ctx = spi_get_drvdata(spi);
> +
> +	drm_panel_detach(&ctx->panel);
> +	drm_panel_remove(&ctx->panel);
> +
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id st7789v_of_match[] = {
> +	{ .compatible = "sitronix,st7789v" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, st7789v_of_match);
> +
> +static struct spi_driver st7789v_driver = {
> +	.probe = st7789v_probe,
> +	.remove = st7789v_remove,
> +	.driver = {
> +		.name = "st7789v",
> +		.of_match_table = st7789v_of_match,
> +	},
> +};
> +module_spi_driver(st7789v_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Sitronix st7789v LCD Driver");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3376 bytes --]

Hi Noralf,

On Fri, Feb 03, 2017 at 07:53:58PM +0100, Noralf Trønnes wrote:
> 
> Den 03.02.2017 10.59, skrev Maxime Ripard:
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >   drivers/gpu/drm/panel/Kconfig                  |   4 +-
> >   drivers/gpu/drm/panel/Makefile                 |   1 +-
> >   drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
> >   3 files changed, 440 insertions(+), 0 deletions(-)
> >   create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > 
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index 62aba976e744..d07b996a07b8 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
> >   	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
> >   	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
> > +config DRM_PANEL_SITRONIX_ST7789V
> > +	tristate "Sitronx ST7789V panel"
> > +	depends on OF && SPI
> > +
> >   endmenu
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index a5c7ec0236e0..41b245d39984 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> >   obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
> >   obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> >   obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> > +obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > new file mode 100644
> > index 000000000000..aab817b55aa6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > @@ -0,0 +1,435 @@
> > +/*
> > + * Copyright (C) 2017 Free Electrons
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License version
> > + * 2 as published by the Free Software Foundation.
> > + */
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_panel.h>
> > +
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#define ST7789V_SLPIN_CMD		0x10
> > +
> > +#define ST7789V_SLPOUT_CMD		0x11
> > +
> > +#define ST7789V_INVON_CMD		0x21
> > +
> > +#define ST7789V_DISPOFF_CMD		0x28
> > +
> > +#define ST7789V_DISPON_CMD		0x29
> > +
> > +#define ST7789V_MADCTL_CMD		0x36
> > +
> > +#define ST7789V_COLMOD_CMD		0x3a
> 
> You can use these from include/video/mipi_display.h:
> 
>         MIPI_DCS_ENTER_SLEEP_MODE       = 0x10,
>         MIPI_DCS_EXIT_SLEEP_MODE        = 0x11,
>         MIPI_DCS_ENTER_INVERT_MODE      = 0x21,
>         MIPI_DCS_SET_DISPLAY_OFF        = 0x28,
>         MIPI_DCS_SET_DISPLAY_ON         = 0x29,
>         MIPI_DCS_SET_ADDRESS_MODE       = 0x36,
>         MIPI_DCS_SET_PIXEL_FORMAT       = 0x3A,

I don't know, that controller cannot do MIPI DSI at all, and we would
have to define all the other commands anyway.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Noralf Trønnes]

Den 06.02.2017 11.50, skrev Maxime Ripard:
> Hi Noralf,
>
> On Fri, Feb 03, 2017 at 07:53:58PM +0100, Noralf Trønnes wrote:
>> Den 03.02.2017 10.59, skrev Maxime Ripard:
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>>    drivers/gpu/drm/panel/Kconfig                  |   4 +-
>>>    drivers/gpu/drm/panel/Makefile                 |   1 +-
>>>    drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
>>>    3 files changed, 440 insertions(+), 0 deletions(-)
>>>    create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>>>
>>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>>> index 62aba976e744..d07b996a07b8 100644
>>> --- a/drivers/gpu/drm/panel/Kconfig
>>> +++ b/drivers/gpu/drm/panel/Kconfig
>>> @@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
>>>    	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
>>>    	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
>>> +config DRM_PANEL_SITRONIX_ST7789V
>>> +	tristate "Sitronx ST7789V panel"
>>> +	depends on OF && SPI
>>> +
>>>    endmenu
>>> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
>>> index a5c7ec0236e0..41b245d39984 100644
>>> --- a/drivers/gpu/drm/panel/Makefile
>>> +++ b/drivers/gpu/drm/panel/Makefile
>>> @@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>>>    obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>>>    obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>>>    obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>>> +obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
>>> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>>> new file mode 100644
>>> index 000000000000..aab817b55aa6
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>>> @@ -0,0 +1,435 @@
>>> +/*
>>> + * Copyright (C) 2017 Free Electrons
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License version
>>> + * 2 as published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <drm/drmP.h>
>>> +#include <drm/drm_panel.h>
>>> +
>>> +#include <linux/gpio/consumer.h>
>>> +#include <linux/spi/spi.h>
>>> +
>>> +#define ST7789V_SLPIN_CMD		0x10
>>> +
>>> +#define ST7789V_SLPOUT_CMD		0x11
>>> +
>>> +#define ST7789V_INVON_CMD		0x21
>>> +
>>> +#define ST7789V_DISPOFF_CMD		0x28
>>> +
>>> +#define ST7789V_DISPON_CMD		0x29
>>> +
>>> +#define ST7789V_MADCTL_CMD		0x36
>>> +
>>> +#define ST7789V_COLMOD_CMD		0x3a
>> You can use these from include/video/mipi_display.h:
>>
>>          MIPI_DCS_ENTER_SLEEP_MODE       = 0x10,
>>          MIPI_DCS_EXIT_SLEEP_MODE        = 0x11,
>>          MIPI_DCS_ENTER_INVERT_MODE      = 0x21,
>>          MIPI_DCS_SET_DISPLAY_OFF        = 0x28,
>>          MIPI_DCS_SET_DISPLAY_ON         = 0x29,
>>          MIPI_DCS_SET_ADDRESS_MODE       = 0x36,
>>          MIPI_DCS_SET_PIXEL_FORMAT       = 0x3A,
> I don't know, that controller cannot do MIPI DSI at all, and we would
> have to define all the other commands anyway.

MIPI DCS commands can be sent over both Display Serial Interface (DSI)
and in your case Display Bus Interface (DBI) (not quite sure if the
Control Interface can be outside of DBI). So Display Command Set (DCS)
is not specific to the interface, but to the controller.

It's up to you if you want to use them though.


Noralf.

Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4292 bytes --]

On Mon, Feb 06, 2017 at 01:13:19PM +0100, Noralf Trønnes wrote:
> 
> Den 06.02.2017 11.50, skrev Maxime Ripard:
> > Hi Noralf,
> > 
> > On Fri, Feb 03, 2017 at 07:53:58PM +0100, Noralf Trønnes wrote:
> > > Den 03.02.2017 10.59, skrev Maxime Ripard:
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > > ---
> > > >    drivers/gpu/drm/panel/Kconfig                  |   4 +-
> > > >    drivers/gpu/drm/panel/Makefile                 |   1 +-
> > > >    drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
> > > >    3 files changed, 440 insertions(+), 0 deletions(-)
> > > >    create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > 
> > > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > > > index 62aba976e744..d07b996a07b8 100644
> > > > --- a/drivers/gpu/drm/panel/Kconfig
> > > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > > @@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
> > > >    	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
> > > >    	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
> > > > +config DRM_PANEL_SITRONIX_ST7789V
> > > > +	tristate "Sitronx ST7789V panel"
> > > > +	depends on OF && SPI
> > > > +
> > > >    endmenu
> > > > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > > > index a5c7ec0236e0..41b245d39984 100644
> > > > --- a/drivers/gpu/drm/panel/Makefile
> > > > +++ b/drivers/gpu/drm/panel/Makefile
> > > > @@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> > > >    obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
> > > >    obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> > > >    obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> > > > +obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > new file mode 100644
> > > > index 000000000000..aab817b55aa6
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > @@ -0,0 +1,435 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Free Electrons
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License version
> > > > + * 2 as published by the Free Software Foundation.
> > > > + */
> > > > +
> > > > +#include <drm/drmP.h>
> > > > +#include <drm/drm_panel.h>
> > > > +
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/spi/spi.h>
> > > > +
> > > > +#define ST7789V_SLPIN_CMD		0x10
> > > > +
> > > > +#define ST7789V_SLPOUT_CMD		0x11
> > > > +
> > > > +#define ST7789V_INVON_CMD		0x21
> > > > +
> > > > +#define ST7789V_DISPOFF_CMD		0x28
> > > > +
> > > > +#define ST7789V_DISPON_CMD		0x29
> > > > +
> > > > +#define ST7789V_MADCTL_CMD		0x36
> > > > +
> > > > +#define ST7789V_COLMOD_CMD		0x3a
> > > You can use these from include/video/mipi_display.h:
> > > 
> > >          MIPI_DCS_ENTER_SLEEP_MODE       = 0x10,
> > >          MIPI_DCS_EXIT_SLEEP_MODE        = 0x11,
> > >          MIPI_DCS_ENTER_INVERT_MODE      = 0x21,
> > >          MIPI_DCS_SET_DISPLAY_OFF        = 0x28,
> > >          MIPI_DCS_SET_DISPLAY_ON         = 0x29,
> > >          MIPI_DCS_SET_ADDRESS_MODE       = 0x36,
> > >          MIPI_DCS_SET_PIXEL_FORMAT       = 0x3A,
> > I don't know, that controller cannot do MIPI DSI at all, and we would
> > have to define all the other commands anyway.
> 
> MIPI DCS commands can be sent over both Display Serial Interface (DSI)
> and in your case Display Bus Interface (DBI) (not quite sure if the
> Control Interface can be outside of DBI). So Display Command Set (DCS)
> is not specific to the interface, but to the controller.
> 
> It's up to you if you want to use them though.

I think it makes sense to unify as much as possible to make it easier to
spot similarities between controllers. Also, we've already got these
definitions in one place, no use in duplicating them. They are obviously
used for the same purposes.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 9404 bytes --]

On Fri, Feb 03, 2017 at 10:59:06AM +0100, Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                  |   4 +-
>  drivers/gpu/drm/panel/Makefile                 |   1 +-
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
>  3 files changed, 440 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 62aba976e744..d07b996a07b8 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
>  	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
>  	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
>  
> +config DRM_PANEL_SITRONIX_ST7789V
> +	tristate "Sitronx ST7789V panel"

"Sitronix"

> +	depends on OF && SPI
> +

Maybe be more verbose here about what kind of panel it is, and what
resolution it provides.

>  endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index a5c7ec0236e0..41b245d39984 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> +obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> new file mode 100644
> index 000000000000..aab817b55aa6
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -0,0 +1,435 @@
> +/*
> + * Copyright (C) 2017 Free Electrons
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/spi/spi.h>

I prefer linux/* headers to go first.

> +
> +#define ST7789V_SLPIN_CMD		0x10
> +
> +#define ST7789V_SLPOUT_CMD		0x11
> +
> +#define ST7789V_INVON_CMD		0x21
> +
> +#define ST7789V_DISPOFF_CMD		0x28
> +
> +#define ST7789V_DISPON_CMD		0x29
> +
> +#define ST7789V_MADCTL_CMD		0x36
> +
> +#define ST7789V_COLMOD_CMD		0x3a
> +#define ST7789V_COLMOD_RGB_FMT_18BITS		(6 << 4)
> +#define ST7789V_COLMOD_CTRL_FMT_18BITS		(6 << 0)

MIPI_DCS_PIXEL_FMT_18BIT?

[...]
> +struct st7789v {
> +	struct drm_panel panel;
> +	struct spi_device *spi;
> +	struct gpio_desc *reset;
> +	struct backlight_device *backlight;
> +};
> +
> +enum st7789v_prefix {
> +	ST7789V_COMMAND = 0,
> +	ST7789V_DATA = 1,
> +};
> +
> +static inline struct st7789v *panel_to_st7789v(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct st7789v, panel);
> +}
> +
> +static int st7789v_spi_write(struct st7789v *ctx, u8 prefix, u8 data)

Why aren't you using the proper type here for prefix?

> +{
> +	struct spi_transfer xfer = { };
> +	struct spi_message msg;
> +	u16 txbuf = ((prefix & 1) << 8) | data;
> +
> +	spi_message_init(&msg);
> +
> +	xfer.tx_buf = &txbuf;
> +	xfer.bits_per_word = 9;
> +	xfer.len = sizeof(txbuf);
> +
> +	spi_message_add_tail(&xfer, &msg);
> +	return spi_sync(ctx->spi, &msg);
> +}
> +
> +static int st7789v_write_command(struct st7789v *ctx, u8 cmd)
> +{
> +	return st7789v_spi_write(ctx, ST7789V_COMMAND, cmd);
> +}
> +
> +static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
> +{
> +	return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
> +}
> +
> +static int st7789v_write_command_data(struct st7789v *ctx, u8 cmd,
> +				      unsigned long n_data, ...)
> +{
> +	va_list ap;
> +	int i;
> +
> +	st7789v_write_command(ctx, cmd);
> +
> +	va_start(ap, n_data);
> +	for (i = 0; i < n_data; i++)
> +		st7789v_write_data(ctx, va_arg(ap, int));
> +	va_end(ap);
> +
> +	return 0;
> +}

Please propagate error codes from st7789v_write_{command,data}(). And
you should abort early on errors.

> +#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
> +#define st7789v_send(ctx, cmd, ...)					\
> +	st7789v_write_command_data(ctx, cmd, NUMARGS(__VA_ARGS__),	\
> +				   ##__VA_ARGS__)

How is this going to work if any of the arguments happens to not be an
int? What if you have something like this:

	u8 value = 0x2;

	st7789v_write_command_data(ctx, cmd, 0x1, value, 0x3);

? Wouldn't that invalidly read "value" as int and wrongly increment the
ap by three bytes too many?

> +static int st7789v_prepare(struct drm_panel *panel)
> +{
> +	struct st7789v *ctx = panel_to_st7789v(panel);
> +
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(30);
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(120);
> +
> +	st7789v_send(ctx, ST7789V_SLPOUT_CMD);
> +
> +	/* We need to wait 120ms after a sleep out command */
> +	msleep(120);
> +
> +	st7789v_send(ctx, ST7789V_MADCTL_CMD, 0);
> +
> +	st7789v_send(ctx, ST7789V_COLMOD_CMD,
> +		     ST7789V_COLMOD_CTRL_FMT_18BITS |
> +		     ST7789V_COLMOD_RGB_FMT_18BITS);
> +
> +	st7789v_send(ctx, ST7789V_PORCTRL_CMD,
> +		     0xc,		/* Backporch, normal mode*/
> +		     0xc,		/* Frontporch, normal mode */
> +		     0,			/* Disable separate porch control */
> +		     ST7789V_PORCTRL_IDLE_BP(3) |
> +		     ST7789V_PORCTRL_IDLE_FP(3),
> +		     ST7789V_PORCTRL_PARTIAL_BP(3) |
> +		     ST7789V_PORCTRL_PARTIAL_FP(3));
> +
> +	st7789v_send(ctx, ST7789V_GCTRL_CMD,
> +		     ST7789V_GCTRL_VGLS(5) | ST7789V_GCTRL_VGHS(3));
> +
> +	st7789v_send(ctx, ST7789V_VCOMS_CMD, 0x2b);
> +
> +	st7789v_send(ctx, ST7789V_LCMCTRL_CMD, ST7789V_LCMCTRL_XMH |
> +		     ST7789V_LCMCTRL_XMX | ST7789V_LCMCTRL_XBGR);
> +
> +	st7789v_send(ctx, ST7789V_VDVVRHEN_CMD, ST7789V_VDVVRHEN_CMDEN);
> +
> +	st7789v_send(ctx, ST7789V_VRHS_CMD, 0xf);
> +
> +	st7789v_send(ctx, ST7789V_VDVS_CMD, 0x20);
> +
> +	st7789v_send(ctx, ST7789V_FRCTRL2_CMD, 0xf);
> +
> +	st7789v_send(ctx, ST7789V_PWCTRL1_CMD, ST7789V_PWCTRL1_MAGIC,
> +		     ST7789V_PWCTRL1_AVDD(2) | ST7789V_PWCTRL1_AVCL(2) |
> +		     ST7789V_PWCTRL1_VDS(1));
> +
> +	st7789v_send(ctx, ST7789V_PVGAMCTRL_CMD,
> +		     ST7789V_PVGAMCTRL_VP63(0xd),
> +		     ST7789V_PVGAMCTRL_VP1(0xca),
> +		     ST7789V_PVGAMCTRL_VP2(0xe),
> +		     ST7789V_PVGAMCTRL_VP4(8),
> +		     ST7789V_PVGAMCTRL_VP6(9),
> +		     ST7789V_PVGAMCTRL_VP13(7),
> +		     ST7789V_PVGAMCTRL_VP20(0x2d),
> +		     ST7789V_PVGAMCTRL_VP27(0xb) | ST7789V_PVGAMCTRL_VP36(3),
> +		     ST7789V_PVGAMCTRL_VP43(0x3d),
> +		     ST7789V_PVGAMCTRL_JP1(3) | ST7789V_PVGAMCTRL_VP50(4),
> +		     ST7789V_PVGAMCTRL_VP57(0xa),
> +		     ST7789V_PVGAMCTRL_VP59(0xa),
> +		     ST7789V_PVGAMCTRL_VP61(0x1b),
> +		     ST7789V_PVGAMCTRL_VP62(0x28));
> +
> +	st7789v_send(ctx, ST7789V_NVGAMCTRL_CMD,
> +		     ST7789V_NVGAMCTRL_VN63(0xd),
> +		     ST7789V_NVGAMCTRL_VN1(0xca),
> +		     ST7789V_NVGAMCTRL_VN2(0xf),
> +		     ST7789V_NVGAMCTRL_VN4(8),
> +		     ST7789V_NVGAMCTRL_VN6(8),
> +		     ST7789V_NVGAMCTRL_VN13(7),
> +		     ST7789V_NVGAMCTRL_VN20(0x2e),
> +		     ST7789V_NVGAMCTRL_VN27(0xc) | ST7789V_NVGAMCTRL_VN36(5),
> +		     ST7789V_NVGAMCTRL_VN43(0x40),
> +		     ST7789V_NVGAMCTRL_JN1(3) | ST7789V_NVGAMCTRL_VN50(4),
> +		     ST7789V_NVGAMCTRL_VN57(9),
> +		     ST7789V_NVGAMCTRL_VN59(0xb),
> +		     ST7789V_NVGAMCTRL_VN61(0x1b),
> +		     ST7789V_NVGAMCTRL_VN62(0x28));
> +
> +	st7789v_send(ctx, ST7789V_INVON_CMD);
> +
> +	st7789v_send(ctx, ST7789V_RAMCTRL_CMD,
> +		     ST7789V_RAMCTRL_DM_RGB | ST7789V_RAMCTRL_RM_RGB,
> +		     ST7789V_RAMCTRL_EPF(3) | ST7789V_RAMCTRL_MAGIC);
> +
> +	st7789v_send(ctx, ST7789V_RGBCTRL_CMD,
> +		     ST7789V_RGBCTRL_WO | ST7789V_RGBCTRL_RCM(2) |
> +		     ST7789V_RGBCTRL_VSYNC_HIGH | ST7789V_RGBCTRL_HSYNC_HIGH |
> +		     ST7789V_RGBCTRL_PCLK_HIGH,
> +		     ST7789V_RGBCTRL_VBP(8), ST7789V_RGBCTRL_HBP(20));
> +
> +	return 0;
> +}

Please check for errors in all of the above.

> +static int st7789v_probe(struct spi_device *spi)
> +{
> +	struct device_node *backlight;
> +	struct st7789v *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(&spi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, ctx);
> +	ctx->spi = spi;
> +
> +	ctx->panel.dev = &spi->dev;
> +	ctx->panel.funcs = &st7789v_drm_funcs;
> +
> +	ctx->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset)) {
> +		dev_err(&spi->dev, "Couldn't get our reset line\n");
> +		return PTR_ERR(ctx->reset);
> +	}
> +
> +	backlight = of_parse_phandle(spi->dev.of_node, "backlight", 0);
> +	if (backlight) {
> +		ctx->backlight = of_find_backlight_by_node(backlight);
> +		of_node_put(backlight);
> +
> +		if (!ctx->backlight)
> +			return -EPROBE_DEFER;
> +	}

This could make use of one of the helpers that Noralf had sent earlier
for TinyDRM. That is, if it weren't specific to TinyDRM.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Mon, Feb 06, 2017 at 02:26:20PM +0100, Thierry Reding wrote:
> > +#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
> > +#define st7789v_send(ctx, cmd, ...)					\
> > +	st7789v_write_command_data(ctx, cmd, NUMARGS(__VA_ARGS__),	\
> > +				   ##__VA_ARGS__)
> 
> How is this going to work if any of the arguments happens to not be an
> int? What if you have something like this:
> 
> 	u8 value = 0x2;
> 
> 	st7789v_write_command_data(ctx, cmd, 0x1, value, 0x3);
> 
> ? Wouldn't that invalidly read "value" as int and wrongly increment the
> ap by three bytes too many?


Hmmm, it might be. I'll remove it, and take all your other changes
into account.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH v3 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

The ARM Mali Utgard GPU family is embedded into a number of SoCs from
Allwinner, Amlogic, Mediatek or Rockchip.

Add a binding for the GPU of that family.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

---
Changes from v2:
  - Drop 0 in the unit-address
  - Add missing st compatible
  - Add Amlogic compatibles

Changes from v1:
  - Dropped the arm,mali-utgard compatible
  - Made the clocks mandatory
  - Added Linus Walleij Reviewed-by, and the ST compatible for the Mali
---
 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 82 ++++++++-
 1 file changed, 82 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
new file mode 100644
index 000000000000..b704a78e492c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
@@ -0,0 +1,82 @@
+ARM Mali Utgard GPU
+===================
+
+Required properties:
+  - compatible
+    * Must be one of the following:
+      + "arm,mali-300"
+      + "arm,mali-400"
+      + "arm,mali-450"
+    * And, optionally, one of the vendor specific compatible:
+      + allwinner,sun4i-a10-mali
+      + allwinner,sun7i-a20-mali
+      + amlogic,meson-gxbb-mali
+      + amlogic,meson-gxl-mali
+      + stericsson,db8500-mali
+
+  - reg: Physical base address and length of the GPU registers
+
+  - interrupts: an entry for each entry in interrupt-names.
+    See ../interrupt-controller/interrupts.txt for details.
+
+  - interrupt-names:
+    * ppX: Pixel Processor X interrupt (X from 0 to 7)
+    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
+    * pp: Pixel Processor broadcast interrupt (mali-450 only)
+    * gp: Geometry Processor interrupt
+    * gpmmu: Geometry Processor MMU interrupt
+
+  - clocks: an entry for each entry in clock-names
+  - clock-names:
+    * bus: bus clock for the GPU
+    * core: clock driving the GPU itself
+
+Optional properties:
+  - interrupt-names and interrupts:
+    * pmu: Power Management Unit interrupt, if implemented in hardware
+
+Vendor-specific bindings
+------------------------
+
+The Mali GPU is integrated very differently from one SoC to
+another. In order to accomodate those differences, you have the option
+to specify one more vendor-specific compatible, among:
+
+  - allwinner,sun4i-a10-mali
+    Required properties:
+      * resets: phandle to the reset line for the GPU
+
+  - allwinner,sun7i-a20-mali
+    Required properties:
+      * resets: phandle to the reset line for the GPU
+
+  - stericsson,db8500-mali
+    Required properties:
+      * interrupt-names and interrupts:
+        + combined: combined interrupt of all of the above lines
+
+Example:
+
+mali: gpu@1c40000 {
+	compatible = "allwinner,sun7i-a20-mali", "arm,mali-400";
+	reg = <0x01c40000 0x10000>;
+	interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+	interrupt-names = "gp",
+			  "gpmmu",
+			  "pp0",
+			  "ppmmu0",
+			  "pp1",
+			  "ppmmu1",
+			  "pmu";
+	clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
+	clock-names = "bus", "core";
+	resets = <&ccu RST_BUS_GPU>;
+};
+
+

base-commit: e24f5ead1d6192d59e5e08327387ee3887820a10
-- 
git-series 0.8.11

[PATCH v3 2/2] ARM: sun8i: dt: Add mali node

[Maxime Ripard]

The A23 and A33 have an ARM Mali 400 GPU. Now that we have a binding, add
it to our DT.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

---

Changes from v2:
  - Removed leading 0 from the unit address
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index d92fdf5a7b26..35008b78d899 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -472,6 +472,32 @@
 			#size-cells = <0>;
 		};
 
+		mali: gpu@1c40000 {
+			compatible = "allwinner,sun8i-a23-mali",
+				     "allwinner,sun7i-a20-mali", "arm,mali-400";
+			reg = <0x01c40000 0x10000>;
+			interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "gp",
+					  "gpmmu",
+					  "pp0",
+					  "ppmmu0",
+					  "pp1",
+					  "ppmmu1",
+					  "pmu";
+			clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
+			clock-names = "bus", "core";
+			resets = <&ccu RST_BUS_GPU>;
+
+			assigned-clocks = <&ccu CLK_GPU>;
+			assigned-clock-rates = <408000000>;
+		};
+
 		gic: interrupt-controller@01c81000 {
 			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
 			reg = <0x01c81000 0x1000>,
-- 
git-series 0.8.11

Re: [PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[Noralf Trønnes]

Den 03.02.2017 10.59, skrev Maxime Ripard:
> Hi,
>
> Here is an attempt at supporting the ST7789V LCD controller from Sitronix.

What happens if there's another panel driven by ST7789V that needs
a different controller initialization?
Maybe it's better to name it after the panel, not the controller.

> It is controlled through an SPI bus, with a twist, since each byte sent
> must be prefixed by a bit, which needs an 9-bits-per-word SPI controller,
> which is quite rare. Else, you would need to bitbang it.

It's possible to support SPI controllers that can't do 9-bit by using
8-bit mode and blocks of 9 bytes to send 8x 9-bit words padded with
zeroes. Zero is MIPI DCS no-op command.

Noralf.

> Let me know what you think,
> Maxime
>
> Maxime Ripard (2):
>    dt-bindings: display: panel: Add bindings for the Sitronix ST7789V panel
>    drm/panel: Add driver for sitronix ST7789V panel
>
>   Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt |  36 ++++++-
>   drivers/gpu/drm/panel/Kconfig                                        |   4 +-
>   drivers/gpu/drm/panel/Makefile                                       |   1 +-
>   drivers/gpu/drm/panel/panel-sitronix-st7789v.c                       | 435 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   4 files changed, 476 insertions(+), 0 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/display/panel/sitronix,st7789v.txt
>   create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>
> base-commit: 2a6e628efb37432a83bb42b00e3c403b1d8873dd

Re: [PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

Hi Noralf,

On Fri, Feb 03, 2017 at 07:48:51PM +0100, Noralf Trønnes wrote:
> 
> Den 03.02.2017 10.59, skrev Maxime Ripard:
> > Hi,
> > 
> > Here is an attempt at supporting the ST7789V LCD controller from Sitronix.
> 
> What happens if there's another panel driven by ST7789V that needs
> a different controller initialization?

You know those panels / controllers much better than I do, but why
would that be the case?

> Maybe it's better to name it after the panel, not the controller.

I guess you could also use that panel directly without the controller?

> > It is controlled through an SPI bus, with a twist, since each byte sent
> > must be prefixed by a bit, which needs an 9-bits-per-word SPI controller,
> > which is quite rare. Else, you would need to bitbang it.
> 
> It's possible to support SPI controllers that can't do 9-bit by using
> 8-bit mode and blocks of 9 bytes to send 8x 9-bit words padded with
> zeroes. Zero is MIPI DCS no-op command.

Ah, yes, that makes sense. Thanks for the hint.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[Noralf Trønnes]

Den 06.02.2017 11.39, skrev Maxime Ripard:
> Hi Noralf,
>
> On Fri, Feb 03, 2017 at 07:48:51PM +0100, Noralf Trønnes wrote:
>> Den 03.02.2017 10.59, skrev Maxime Ripard:
>>> Hi,
>>>
>>> Here is an attempt at supporting the ST7789V LCD controller from Sitronix.
>> What happens if there's another panel driven by ST7789V that needs
>> a different controller initialization?
> You know those panels / controllers much better than I do, but why
> would that be the case?
>
>> Maybe it's better to name it after the panel, not the controller.
> I guess you could also use that panel directly without the controller?

A controller can drive many different panels that can require different
initializations. I faced that with staging/fbtft, when I wrote
controller drivers having initialization code, and then came across
displays with the same controller but with a different initialization.

Trying to write controller drivers for these controllers is very
difficult with all the possible permutations. On top of that we have
those undocumented commands/registers.

Some panels come with embedded controllers, in which case it makes
sense to write a driver for the panel.

But if the panel and controller are separate, then I don't know. Maybe
the chance of coming across two uncompatible ST7789V and panel
combinations in drm/panel is extremly low.


Noralf.

Re: [PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

On Mon, Feb 06, 2017 at 12:29:31PM +0100, Noralf Trønnes wrote:
> 
> Den 06.02.2017 11.39, skrev Maxime Ripard:
> > Hi Noralf,
> > 
> > On Fri, Feb 03, 2017 at 07:48:51PM +0100, Noralf Trønnes wrote:
> > > Den 03.02.2017 10.59, skrev Maxime Ripard:
> > > > Hi,
> > > > 
> > > > Here is an attempt at supporting the ST7789V LCD controller from Sitronix.
> > > What happens if there's another panel driven by ST7789V that needs
> > > a different controller initialization?
> > You know those panels / controllers much better than I do, but why
> > would that be the case?
> > 
> > > Maybe it's better to name it after the panel, not the controller.
> > I guess you could also use that panel directly without the controller?
> 
> A controller can drive many different panels that can require different
> initializations. I faced that with staging/fbtft, when I wrote
> controller drivers having initialization code, and then came across
> displays with the same controller but with a different initialization.
> 
> Trying to write controller drivers for these controllers is very
> difficult with all the possible permutations. On top of that we have
> those undocumented commands/registers.
> 
> Some panels come with embedded controllers, in which case it makes
> sense to write a driver for the panel.
> 
> But if the panel and controller are separate, then I don't know. Maybe
> the chance of coming across two uncompatible ST7789V and panel
> combinations in drm/panel is extremly low.

Hmm, I see. If we ever come across that case, I guess we could just
add new optional properties to override the current sequence.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/2] drm/panel: Add support for the Sitronix ST7789V

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

On Tue, Feb 07, 2017 at 04:34:44PM +0100, Maxime Ripard wrote:
> On Mon, Feb 06, 2017 at 12:29:31PM +0100, Noralf Trønnes wrote:
> > 
> > Den 06.02.2017 11.39, skrev Maxime Ripard:
> > > Hi Noralf,
> > > 
> > > On Fri, Feb 03, 2017 at 07:48:51PM +0100, Noralf Trønnes wrote:
> > > > Den 03.02.2017 10.59, skrev Maxime Ripard:
> > > > > Hi,
> > > > > 
> > > > > Here is an attempt at supporting the ST7789V LCD controller from Sitronix.
> > > > What happens if there's another panel driven by ST7789V that needs
> > > > a different controller initialization?
> > > You know those panels / controllers much better than I do, but why
> > > would that be the case?
> > > 
> > > > Maybe it's better to name it after the panel, not the controller.
> > > I guess you could also use that panel directly without the controller?
> > 
> > A controller can drive many different panels that can require different
> > initializations. I faced that with staging/fbtft, when I wrote
> > controller drivers having initialization code, and then came across
> > displays with the same controller but with a different initialization.
> > 
> > Trying to write controller drivers for these controllers is very
> > difficult with all the possible permutations. On top of that we have
> > those undocumented commands/registers.
> > 
> > Some panels come with embedded controllers, in which case it makes
> > sense to write a driver for the panel.
> > 
> > But if the panel and controller are separate, then I don't know. Maybe
> > the chance of coming across two uncompatible ST7789V and panel
> > combinations in drm/panel is extremly low.
> 
> Hmm, I see. If we ever come across that case, I guess we could just
> add new optional properties to override the current sequence.

I think if that ever happens it's probably best to split out ST7789V
code into a helper library that has functions which take parameters.
Then we can simply call those functions with parameters specific to
a panel from a panel-specific driver.

Trying to fit all that into device tree properties is likely going to
end up being really messy.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]