[PATCH 0/2] staging: fbtft: add support for ST7789V and C-Berry28

[PATCH 0/2] staging: fbtft: add support for ST7789V and C-Berry28

[Dennis Menschel]

This set of patches extends the fbtft driver by the following two things,

- support for the ST7789V display controller from Sitronix and
- support for a concrete display which uses this controller, namely the
  C-Berry28, a 2.8" color display from admatec for the Raspberry Pi.

Dennis Menschel (2):
  staging: fbtft: add support for ST7789V display controller
  staging: fbtft: add support for C-Berry28 display

 drivers/staging/fbtft/Kconfig        |  10 ++
 drivers/staging/fbtft/Makefile       |   1 +
 drivers/staging/fbtft/fb_st7789v.c   | 290 +++++++++++++++++++++++++++++++++++
 drivers/staging/fbtft/fbtft_device.c |  74 +++++++++
 4 files changed, 375 insertions(+)
 create mode 100644 drivers/staging/fbtft/fb_st7789v.c

-- 
2.1.4

[PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Dennis Menschel]

This patch adds support for the Sitronix ST7789V display controller.
The controller is intended for small color displays with a resolution
of up to 320x240 pixels.

Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
---
 drivers/staging/fbtft/Kconfig      |  10 ++
 drivers/staging/fbtft/Makefile     |   1 +
 drivers/staging/fbtft/fb_st7789v.c | 290 +++++++++++++++++++++++++++++++++++++
 3 files changed, 301 insertions(+)
 create mode 100644 drivers/staging/fbtft/fb_st7789v.c

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index d473010..883ff5b 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -141,6 +141,16 @@ config FB_TFT_ST7735R
 	help
 	  Generic Framebuffer support for ST7735R
 
+config FB_TFT_ST7789V
+	tristate "FB driver for the ST7789V LCD Controller"
+	depends on FB_TFT
+	help
+	  This enables generic framebuffer support for the Sitronix ST7789V
+	  display controller. The controller is intended for small color
+	  displays with a resolution of up to 320x240 pixels.
+
+	  Say Y if you have such a display that utilizes this controller.
+
 config FB_TFT_TINYLCD
 	tristate "FB driver for tinylcd.com display"
 	depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index b26efdc..4f9071d 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_FB_TFT_SSD1306)     += fb_ssd1306.o
 obj-$(CONFIG_FB_TFT_SSD1331)     += fb_ssd1331.o
 obj-$(CONFIG_FB_TFT_SSD1351)     += fb_ssd1351.o
 obj-$(CONFIG_FB_TFT_ST7735R)     += fb_st7735r.o
+obj-$(CONFIG_FB_TFT_ST7789V)     += fb_st7789v.o
 obj-$(CONFIG_FB_TFT_TINYLCD)     += fb_tinylcd.o
 obj-$(CONFIG_FB_TFT_TLS8204)     += fb_tls8204.o
 obj-$(CONFIG_FB_TFT_UC1611)      += fb_uc1611.o
diff --git a/drivers/staging/fbtft/fb_st7789v.c b/drivers/staging/fbtft/fb_st7789v.c
new file mode 100644
index 0000000..dc7d304
--- /dev/null
+++ b/drivers/staging/fbtft/fb_st7789v.c
@@ -0,0 +1,290 @@
+/*
+ * FB driver for the ST7789V LCD Controller
+ *
+ * Copyright (C) 2015 Dennis Menschel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "fbtft.h"
+
+#define DRVNAME "fb_st7789v"
+
+#define DEFAULT_GAMMA \
+	"70 2C 2E 15 10 09 48 33 53 0B 19 18 20 25\n" \
+	"70 2C 2E 15 10 09 48 33 53 0B 19 18 20 25"
+
+/**
+ * enum st7789v_command - ST7789V display controller commands
+ *
+ * @SLPOUT: sleep out
+ * @DISPOFF: display off
+ * @DISPON: display on
+ * @CASET: column address set
+ * @RASET: row address set
+ * @RAMRW: memory write
+ * @MADCTL: memory data access control
+ * @COLMOD: interface pixel format
+ * @PORCTRL: porch setting
+ * @GCTRL: gate control
+ * @VCOMS: VCOM setting
+ * @VDVVRHEN: VDV and VRH command enable
+ * @VRHS: VRH set
+ * @VDVS: VDV set
+ * @VCMOFSET: VCOM offset set
+ * @PWCTRL1: power control 1
+ * @PVGAMCTRL: positive voltage gamma control
+ * @NVGAMCTRL: negative voltage gamma control
+ *
+ * The command names are the same as those found in the datasheet to ease
+ * looking up their semantics and usage.
+ *
+ * Note that the ST7789V display controller offers quite a few more commands
+ * which have been omitted from this list as they are not used at the moment.
+ */
+enum st7789v_command {
+	SLPOUT = 0x11,
+	DISPOFF = 0x28,
+	DISPON = 0x29,
+	CASET = 0x2A,
+	RASET = 0x2B,
+	RAMRW = 0x2C,
+	MADCTL = 0x36,
+	COLMOD = 0x3A,
+	PORCTRL = 0xB2,
+	GCTRL = 0xB7,
+	VCOMS = 0xBB,
+	VDVVRHEN = 0xC2,
+	VRHS = 0xC3,
+	VDVS = 0xC4,
+	VCMOFSET = 0xC5,
+	PWCTRL1 = 0xD0,
+	PVGAMCTRL = 0xE0,
+	NVGAMCTRL = 0xE1,
+};
+
+#define MADCTL_BGR BIT(3) /* bitmask for RGB/BGR order */
+#define MADCTL_MV BIT(5) /* bitmask for page/column order */
+#define MADCTL_MX BIT(6) /* bitmask for column address order */
+#define MADCTL_MY BIT(7) /* bitmask for page address order */
+
+/**
+ * default_init_sequence - default initialization sequence for ST7789V
+ *
+ * Most of the commands in this init sequence set their parameters to the
+ * same default values which are already in place after the display has been
+ * powered up. (The main exception to this rule is the pixel format which
+ * would default to 18 instead of 16 bit per pixel.)
+ * Nonetheless, this sequence can be used as a template for concrete
+ * displays which usually need some adjustments.
+ */
+static int default_init_sequence[] = {
+	/* turn off sleep mode */
+	-1, SLPOUT,
+	-2, 120,
+
+	/* set pixel format to RGB-565 */
+	-1, COLMOD, 0x05,
+
+	-1, PORCTRL, 0x08, 0x08, 0x00, 0x22, 0x22,
+
+	/*
+	 * VGH = 13.26V
+	 * VGL = -10.43V
+	 */
+	-1, GCTRL, 0x35,
+
+	/*
+	 * VDV and VRH register values come from command write
+	 * (instead of NVM)
+	 */
+	-1, VDVVRHEN, 0x01, 0xFF,
+
+	/*
+	 * VAP =  4.1V + (VCOM + VCOM offset + 0.5 * VDV)
+	 * VAN = -4.1V + (VCOM + VCOM offset + 0.5 * VDV)
+	 */
+	-1, VRHS, 0x0B,
+
+	/* VDV = 0V */
+	-1, VDVS, 0x20,
+
+	/* VCOM = 0.9V */
+	-1, VCOMS, 0x20,
+
+	/* VCOM offset = 0V */
+	-1, VCMOFSET, 0x20,
+
+	/*
+	 * AVDD = 6.8V
+	 * AVCL = -4.8V
+	 * VDS = 2.3V
+	 */
+	-1, PWCTRL1, 0xA4, 0xA1,
+
+	-1, DISPON,
+
+	-3,
+};
+
+/**
+ * set_addr_win() - configure display area to use
+ *
+ * @par: FBTFT parameter object
+ * @xs: first active pixel of x-axis
+ * @ys: first active pixel of y-axis
+ * @xe: last active pixel of x-axis
+ * @ye: last active pixel of y-axis
+ */
+static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
+{
+	write_reg(par, CASET, xs >> 8, xs & 0xFF, xe >> 8, xe & 0xFF);
+	write_reg(par, RASET, ys >> 8, ys & 0xFF, ye >> 8, ye & 0xFF);
+	write_reg(par, RAMRW);
+}
+
+/**
+ * set_var() - apply LCD properties like rotation and BGR mode
+ *
+ * @par: FBTFT parameter object
+ *
+ * Return: 0 on success, < 0 if error occurred.
+ */
+static int set_var(struct fbtft_par *par)
+{
+	u8 madctl_par = 0;
+
+	if (par->bgr)
+		madctl_par |= MADCTL_BGR;
+	switch (par->info->var.rotate) {
+	case 0:
+		break;
+	case 90:
+		madctl_par |= (MADCTL_MV | MADCTL_MY);
+		break;
+	case 180:
+		madctl_par |= (MADCTL_MX | MADCTL_MY);
+		break;
+	case 270:
+		madctl_par |= (MADCTL_MV | MADCTL_MX);
+		break;
+	default:
+		return -EINVAL;
+	}
+	write_reg(par, MADCTL, madctl_par);
+	return 0;
+}
+
+/**
+ * set_gamma() - set gamma curves
+ *
+ * @par: FBTFT parameter object
+ * @curves: gamma curves
+ *
+ * Before the gamma curves are applied, they are preprocessed with a bitmask
+ * to ensure syntactically correct input for the display controller.
+ * This implies that the curves input parameter might be changed by this
+ * function and that illegal gamma values are auto-corrected and not
+ * reported as errors.
+ *
+ * Return: 0 on success, < 0 if error occurred.
+ */
+static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+{
+	int i;
+	int j;
+	int c; /* curve index offset */
+
+	/*
+	 * Bitmasks for gamma curve command parameters.
+	 * The masks are the same for both positive and negative voltage
+	 * gamma curves.
+	 */
+	const u8 gamma_par_mask[] = {
+		0xFF, /* V63[3:0], V0[3:0]*/
+		0x3F, /* V1[5:0] */
+		0x3F, /* V2[5:0] */
+		0x1F, /* V4[4:0] */
+		0x1F, /* V6[4:0] */
+		0x3F, /* J0[1:0], V13[3:0] */
+		0x7F, /* V20[6:0] */
+		0x77, /* V36[2:0], V27[2:0] */
+		0x7F, /* V43[6:0] */
+		0x3F, /* J1[1:0], V50[3:0] */
+		0x1F, /* V57[4:0] */
+		0x1F, /* V59[4:0] */
+		0x3F, /* V61[5:0] */
+		0x3F, /* V62[5:0] */
+	};
+
+	for (i = 0; i < par->gamma.num_curves; i++) {
+		c = i * par->gamma.num_values;
+		for (j = 0; j < par->gamma.num_values; j++)
+			curves[c + j] &= gamma_par_mask[j];
+		write_reg(
+			par, PVGAMCTRL + i,
+			curves[c + 0], curves[c + 1], curves[c + 2],
+			curves[c + 3], curves[c + 4], curves[c + 5],
+			curves[c + 6], curves[c + 7], curves[c + 8],
+			curves[c + 9], curves[c + 10], curves[c + 11],
+			curves[c + 12], curves[c + 13]);
+	}
+	return 0;
+}
+
+/**
+ * blank() - blank the display
+ *
+ * @par: FBTFT parameter object
+ * @on: whether to enable or disable blanking the display
+ *
+ * Return: 0 on success, < 0 if error occurred.
+ */
+static int blank(struct fbtft_par *par, bool on)
+{
+	if (on)
+		write_reg(par, DISPOFF);
+	else
+		write_reg(par, DISPON);
+	return 0;
+}
+
+static struct fbtft_display display = {
+	.regwidth = 8,
+	.width = 240,
+	.height = 320,
+	.init_sequence = default_init_sequence,
+	.gamma_num = 2,
+	.gamma_len = 14,
+	.gamma = DEFAULT_GAMMA,
+	.fbtftops = {
+		.set_addr_win = set_addr_win,
+		.set_var = set_var,
+		.set_gamma = set_gamma,
+		.blank = blank,
+	},
+};
+
+FBTFT_REGISTER_DRIVER(DRVNAME, "sitronix,st7789v", &display);
+
+MODULE_ALIAS("spi:" DRVNAME);
+MODULE_ALIAS("platform:" DRVNAME);
+MODULE_ALIAS("spi:st7789v");
+MODULE_ALIAS("platform:st7789v");
+
+MODULE_DESCRIPTION("FB driver for the ST7789V LCD Controller");
+MODULE_AUTHOR("Dennis Menschel");
+MODULE_LICENSE("GPL");
-- 
2.1.4

[PATCH 2/2] staging: fbtft: add support for C-Berry28 display

[Dennis Menschel]

This patch adds support for the C-Berry28, a small 2.8" color display
with a resolution of 320x240 pixels from admatec for the Raspberry Pi.
It uses the Sitronix ST7789V display controller along with a custom
init sequence and custom gamma curves.

Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
---
 drivers/staging/fbtft/fbtft_device.c | 74 ++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft_device.c b/drivers/staging/fbtft/fbtft_device.c
index a60f389..0dc0e62 100644
--- a/drivers/staging/fbtft/fbtft_device.c
+++ b/drivers/staging/fbtft/fbtft_device.c
@@ -126,6 +126,59 @@ static void adafruit18_green_tab_set_addr_win(struct fbtft_par *par,
 		"02 1c 07 12 37 32 29 2d 29 25 2B 39 00 01 03 10\n" \
 		"03 1d 07 06 2E 2C 29 2D 2E 2E 37 3F 00 00 02 10"
 
+#define CBERRY28_GAMMA \
+		"D0 00 14 15 13 2C 42 43 4E 09 16 14 18 21\n" \
+		"D0 00 14 15 13 0B 43 55 53 0C 17 14 23 20"
+
+static int cberry28_init_sequence[] = {
+	/* turn off sleep mode */
+	-1, 0x11,
+	-2, 120,
+
+	/* set pixel format to RGB-565 */
+	-1, 0x3A, 0x05,
+
+	-1, 0xB2, 0x0C, 0x0C, 0x00, 0x33, 0x33,
+
+	/*
+	 * VGH = 13.26V
+	 * VGL = -10.43V
+	 */
+	-1, 0xB7, 0x35,
+
+	/*
+	 * VDV and VRH register values come from command write
+	 * (instead of NVM)
+	 */
+	-1, 0xC2, 0x01, 0xFF,
+
+	/*
+	 * VAP =  4.7V + (VCOM + VCOM offset + 0.5 * VDV)
+	 * VAN = -4.7V + (VCOM + VCOM offset + 0.5 * VDV)
+	 */
+	-1, 0xC3, 0x17,
+
+	/* VDV = 0V */
+	-1, 0xC4, 0x20,
+
+	/* VCOM = 0.875V */
+	-1, 0xBB, 0x1F,
+
+	/* VCOM offset = 0V */
+	-1, 0xC5, 0x20,
+
+	/*
+	 * AVDD = 6.6V
+	 * AVCL = -4.8V
+	 * VDS = 2.3V
+	 */
+	-1, 0xD0, 0xA4, 0x61,
+
+	-1, 0x29,
+
+	-3,
+};
+
 static int hy28b_init_sequence[] = {
 	-1, 0x00e7, 0x0010, -1, 0x0000, 0x0001,
 	-1, 0x0001, 0x0100, -1, 0x0002, 0x0700,
@@ -313,6 +366,27 @@ static struct fbtft_device_display displays[] = {
 			}
 		}
 	}, {
+		.name = "admatec_c-berry28",
+		.spi = &(struct spi_board_info) {
+			.modalias = "fb_st7789v",
+			.max_speed_hz = 48000000,
+			.mode = SPI_MODE_0,
+			.platform_data = &(struct fbtft_platform_data) {
+				.display = {
+					.buswidth = 8,
+					.backlight = 1,
+					.init_sequence = cberry28_init_sequence,
+				},
+				.gpios = (const struct fbtft_gpio []) {
+					{ "reset", 25 },
+					{ "dc", 22 },
+					{ "led", 18 },
+					{},
+				},
+				.gamma = CBERRY28_GAMMA,
+			}
+		}
+	}, {
 		.name = "agm1264k-fl",
 		.pdev = &(struct platform_device) {
 			.name = "fb_agm1264k-fl",
-- 
2.1.4

Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Noralf Trønnes]

Den 07.10.2015 22:15, skrev Dennis Menschel:
> This patch adds support for the Sitronix ST7789V display controller.
> The controller is intended for small color displays with a resolution
> of up to 320x240 pixels.
>
> Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
> ---

The future of fbtft is uncertain after fbdev maintainer Tomi Valkeinen
suggested that it be removed [1], since fbdev is a deprecated framework.
I have started to look at a DRM version, but this will take some time pull
through (fbtft is just one of my hobby projects). I suggest that we still
accept new drivers and if necessary convert them to DRM drivers later.

If we do that, there's one change I would like to see and that is switching
to display/panel drivers instead of controller drivers. The only common
functionality between controllers, that we make use of, is the
set_addr_win() function (I didn't know that when I started fbtft based on
the 2 displays I had). This is a small function so I don't see a problem
duplicating this in new drivers. It's also possible to export the function
from a controller driver. fbtft has a default set_addr_win() implementation
that matches the MIPI compatible controllers (0x2A, 0x2B, 0x2C).
(blank() is used on OLED controllers and set_gamma() will be obsolete with
display drivers since gamma can be set in init())

Whatever fbtft turns into, it will use display drivers instead of
controller drivers. I have done some work on fbtft that gives a hint on
how it might look [2] (I stopped that work when I understood that fbtft
might not move out of staging after all).
The main reason for switching to displays drivers is that most controllers
have a quite large register set with lots of settings. This is quite
impossible to turn into Device Tree properties. And many panels also use
undocumented registers.

fbtft_device
The fbtft_device module provides a way to add devices for the fbtft drivers.
It was made to make life simple for Raspberry Pi users when it's kernel
didn't support Device Tree. There are no other modules like it in the 
kernel,
so it will not move out of staging. The reason I haven't removed it is that
it's tightly coupled with the flexfb driver making it usable with panels
that's not supported by the other drivers. For instance the set_par()
function, in the drivers that implement it, hardcodes some rotation/mirror
register values that are panel specific. flexfb doesn't have a set_par()
function, so it won't overwrite values set in init(), making it useful
with some panels. I suggest that we don't make any more additions to
fbtft_device, but just keep it as it is for now. fbtft_device has many
module parameters that should cover most if not all configs [3].


So for this particular patch I suggest you turn it into a display driver
and that we merge that (drop the fbtft_device patch).
Please write the init sequence in an init() function using write_reg()
since init_sequence will go away. There's an enum for the MIPI DCS
commands in include/video/mipi_display.h


Noralf.

[1] https://lkml.org/lkml/2015/9/24/253
[2] https://github.com/notro/linux-staging/commits/next
[3] https://github.com/notro/fbtft/wiki/fbtft_device

Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Dennis Menschel]

Am 10.10.2015 um 17:36 schrieb Noralf Trønnes:
> 
> Den 07.10.2015 22:15, skrev Dennis Menschel:
>> This patch adds support for the Sitronix ST7789V display controller.
>> The controller is intended for small color displays with a resolution
>> of up to 320x240 pixels.
>>
>> Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
>> ---
> 
> The future of fbtft is uncertain after fbdev maintainer Tomi Valkeinen
> suggested that it be removed [1], since fbdev is a deprecated framework.
> I have started to look at a DRM version, but this will take some time pull
> through (fbtft is just one of my hobby projects). I suggest that we still
> accept new drivers and if necessary convert them to DRM drivers later.
> 
> If we do that, there's one change I would like to see and that is switching
> to display/panel drivers instead of controller drivers. The only common
> functionality between controllers, that we make use of, is the
> set_addr_win() function (I didn't know that when I started fbtft based on
> the 2 displays I had). This is a small function so I don't see a problem
> duplicating this in new drivers. It's also possible to export the function
> from a controller driver. fbtft has a default set_addr_win() implementation
> that matches the MIPI compatible controllers (0x2A, 0x2B, 0x2C).
> (blank() is used on OLED controllers and set_gamma() will be obsolete with
> display drivers since gamma can be set in init())
> 
> Whatever fbtft turns into, it will use display drivers instead of
> controller drivers. I have done some work on fbtft that gives a hint on
> how it might look [2] (I stopped that work when I understood that fbtft
> might not move out of staging after all).
> The main reason for switching to displays drivers is that most controllers
> have a quite large register set with lots of settings. This is quite
> impossible to turn into Device Tree properties. And many panels also use
> undocumented registers.
> 
> fbtft_device
> The fbtft_device module provides a way to add devices for the fbtft
> drivers.
> It was made to make life simple for Raspberry Pi users when it's kernel
> didn't support Device Tree. There are no other modules like it in the
> kernel,
> so it will not move out of staging. The reason I haven't removed it is that
> it's tightly coupled with the flexfb driver making it usable with panels
> that's not supported by the other drivers. For instance the set_par()
> function, in the drivers that implement it, hardcodes some rotation/mirror
> register values that are panel specific. flexfb doesn't have a set_par()
> function, so it won't overwrite values set in init(), making it useful
> with some panels. I suggest that we don't make any more additions to
> fbtft_device, but just keep it as it is for now. fbtft_device has many
> module parameters that should cover most if not all configs [3].
> 
> 
> So for this particular patch I suggest you turn it into a display driver
> and that we merge that (drop the fbtft_device patch).
> Please write the init sequence in an init() function using write_reg()
> since init_sequence will go away. There's an enum for the MIPI DCS
> commands in include/video/mipi_display.h
> 
> 
> Noralf.
> 
> [1] https://lkml.org/lkml/2015/9/24/253
> [2] https://github.com/notro/linux-staging/commits/next
> [3] https://github.com/notro/fbtft/wiki/fbtft_device
> 

Thank you for your detailed explanation about the current state of fbtft
and the future plans for a possible successor framework. As suggested by
you, I'll make the following changes in the next patch:

- Change the st7789v controller driver to a cberry28 display driver.
- If applicable, use the default fbtft implementation of set_addr_win().
- Remove the blank() function as it is only intended for OLED displays.
- Use an init() function instead of an init sequence.
- Integrate the gamma settings into the init() function.
- Use enum for MIPI DCS commands which can be found in
  include/video/mipi_display.h.
- Do not extend fbtft_device since the displays can now be loaded via
  device tree (overlays).

Best regards,
Dennis Menschel

Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Noralf Trønnes]

Den 11.10.2015 09:31, skrev Dennis Menschel:
> Am 10.10.2015 um 17:36 schrieb Noralf Trønnes:
>> Den 07.10.2015 22:15, skrev Dennis Menschel:
>>> This patch adds support for the Sitronix ST7789V display controller.
>>> The controller is intended for small color displays with a resolution
>>> of up to 320x240 pixels.
>>>
>>> Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
>>> ---

...

>> (blank() is used on OLED controllers and set_gamma() will be obsolete with
>> display drivers since gamma can be set in init())

...

> Thank you for your detailed explanation about the current state of fbtft
> and the future plans for a possible successor framework. As suggested by
> you, I'll make the following changes in the next patch:
>
> - Change the st7789v controller driver to a cberry28 display driver.
> - If applicable, use the default fbtft implementation of set_addr_win().
> - Remove the blank() function as it is only intended for OLED displays.

I want to expand a bit on blank(), it can be used by all drivers, but all
the non-OLED displays I have tried turns off the pixels when it's blanked,
so the backlight shines through, making it of no use (no power savings to
speak of either). So these displays need the backlight to be turned off
during blanking. The main reason blank() isn't implemented in the
controller drivers, is that if it's used on a display without backlight
control, the display would turn white during blanking.

There's a bug in the fbtft backlight implementation that prevents it from
turning off backlight on the first fb_blank. Subsequent blanks are ok.
I haven't looked into it, because the fbtft backlight implementation
should really be handled by the gpio-backlight driver. That's what I've
done in my new work.


Noralf.

Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Dennis Menschel]

Am 11.10.2015 um 16:19 schrieb Noralf Trønnes:
> 
> Den 11.10.2015 09:31, skrev Dennis Menschel:
>> Am 10.10.2015 um 17:36 schrieb Noralf Trønnes:
>>> Den 07.10.2015 22:15, skrev Dennis Menschel:
>>>> This patch adds support for the Sitronix ST7789V display controller.
>>>> The controller is intended for small color displays with a resolution
>>>> of up to 320x240 pixels.
>>>>
>>>> Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
>>>> ---
> 
> ...
> 
>>> (blank() is used on OLED controllers and set_gamma() will be obsolete
>>> with
>>> display drivers since gamma can be set in init())
> 
> ...
> 
>> Thank you for your detailed explanation about the current state of fbtft
>> and the future plans for a possible successor framework. As suggested by
>> you, I'll make the following changes in the next patch:
>>
>> - Change the st7789v controller driver to a cberry28 display driver.
>> - If applicable, use the default fbtft implementation of set_addr_win().
>> - Remove the blank() function as it is only intended for OLED displays.
> 
> I want to expand a bit on blank(), it can be used by all drivers, but all
> the non-OLED displays I have tried turns off the pixels when it's blanked,
> so the backlight shines through, making it of no use (no power savings to
> speak of either). So these displays need the backlight to be turned off
> during blanking. The main reason blank() isn't implemented in the
> controller drivers, is that if it's used on a display without backlight
> control, the display would turn white during blanking.
> 
> There's a bug in the fbtft backlight implementation that prevents it from
> turning off backlight on the first fb_blank. Subsequent blanks are ok.
> I haven't looked into it, because the fbtft backlight implementation
> should really be handled by the gpio-backlight driver. That's what I've
> done in my new work.
> 
> 
> Noralf.

Hello Noralf,

after having seen that my previous patches have already moved to
staging-next, I've reconsidered your recent suggestions.

If I rewrote the ST7789V display controller driver into a C-Berry28
display driver, the last patches would be effectively reverted,
rendering previous testing efforts in vain (among other things). The
driver would loose much of its current flexibility (i.e. being able to
support similar displays with the same controller but different voltages
or gamma curves) and the meta data concerning the ST7789V would be
thrown away. Furthermore, as the future direction of fbtft development
is uncertain, it makes little sense to optimize the code for an
interface which has not been defined yet. Even if the final interface to
the normal user should provide display drivers instead of display
controller drivers, the abstraction of display controllers as reusable
components for display driver developers would still be a desirable
solution. In addition, the more the available drivers for fbtft diverge
from each other in terms of design and structure, the harder it will
become to convert them to the successor framework in the future.

Therefore, I think it is reasonable to keep the current display
controller driver for the ST7789V.

Best regards,
Dennis Menschel