drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

This adds a new binding for Himax HX8357D display panels. It includes a compatible string for one display (more can be added in the future).

The YX350HV15 panel[1] is found in the Adafruit PiTFT 3.5" Touch Screen for Raspberry Pi.

[1] https://learn.adafruit.com/adafruit-pitft-3-dot-5-touch-screen-for-raspberry-pi/downloads

This binding is closely modeled after the ili9341 binding, for a similar product from adafruit. The primary difference is that the hx8367d doesn't have a reset line that I can find in the schematics.

Signed-off-by Eric Anholt <[email protected]>

---
 .../bindings/display/himax,hx8357d.txt        | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/himax,hx8357d.txt

Documentation/devicetree/bindings/display/himax,hx8357d.txt new file +0 -0

new file mode 100644
index 000000000000..48586e570be0

Documentation/devicetree/bindings/display/himax,hx8357d.txt new file +17 -0

+
+The node for this driver must be a child node of a SPI controller, hence
+all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+Optional properties:
+- rotation:	panel rotation in degrees counter clockwise (0,90,180,270)
+- backlight:	phandle of the backlight device attached to the panel
+
+Example:
+	display@0{
+		compatible = "adafruit,yx350hv15", "himax,hx8357d";
+		reg = <0>;
+		spi-max-frequency = <32000000>;
+		dc-gpios = <&gpio0 25 GPIO_ACTIVE_HIGH>;
+		rotation = <90>;
+		backlight = <&backlight>;
+	};

Signature · 6 lines

2.19.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

This adds a new binding for Himax HX8357D display panels. It includes a compatible string for one display (more can be added in the future).

The YX350HV15 panel[1] is found in the Adafruit PiTFT 3.5" Touch Screen for Raspberry Pi.

[1] https://learn.adafruit.com/adafruit-pitft-3-dot-5-touch-screen-for-raspberry-pi/downloads

This binding is closely modeled after the ili9341 binding, for a similar product from adafruit. The primary difference is that the hx8367d doesn't have a reset line that I can find in the schematics.

Signed-off-by Eric Anholt <[email protected]>

---
  .../bindings/display/himax,hx8357d.txt        | 25 +++++++++++++++++++
  1 file changed, 25 insertions(+)

create mode 100644 Documentation/devicetree/bindings/display/himax,hx8357d.txt

Documentation/devicetree/bindings/display/himax,hx8357d.txt new file +15 -0

new file mode 100644
index 000000000000..48586e570be0
@@ -0,0 +1,25 @@
+Himax HX8357D display panels
+
+This binding is for display panels using a Himax HX8357D controller in SPI
+mode, such as the Adafruit 3.5" TFT for Raspberry Pi.
+
+Required properties:
+- compatible:	"adafruit,yx350hv15", "himax,hx8357d"
+- dc-gpios:	D/C pin
+
+The node for this driver must be a child node of a SPI controller, hence
+all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+Optional properties:
+- rotation:	panel rotation in degrees counter clockwise (0,90,180,270)
+- backlight:	phandle of the backlight device attached to the panel

I want to sort out support for tinydrm in vc4, so I needed to get a tinydrm-appropriate panel working and this is what I had on hand. This is derived from a combination of ili9341.c from tinydrm and fb_hx8357d.c from staging's fbtft. The register header is copied directly from staging's fbtft, on the assumption that we will delete that copy later.

Signed-off-by Eric Anholt <[email protected]>

---
  MAINTAINERS                       |   7 +
  drivers/gpu/drm/tinydrm/Kconfig   |  11 ++
  drivers/gpu/drm/tinydrm/Makefile  |   1 +
  drivers/gpu/drm/tinydrm/hx8357d.c | 261 ++++++++++++++++++++++++++++++
  drivers/gpu/drm/tinydrm/hx8357d.h |  71 ++++++++
  5 files changed, 351 insertions(+)

create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.c create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.h

MAINTAINERS +7 -0

index 39c3f6682ace..e78971e20a11 100644
@@ -4623,6 +4623,13 @@ S:	Maintained
  F:	drivers/gpu/drm/tinydrm/ili9225.c
  F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR HX8357D PANELS
+M:	Eric Anholt <[email protected]>
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+S:	Maintained
+F:	drivers/gpu/drm/tinydrm/hx8357d.c
+F:	Documentation/devicetree/bindings/display/himax,hx8357d.txt
+
  DRM DRIVER FOR INTEL I810 VIDEO CARDS
  S:	Orphan / Obsolete
  F:	drivers/gpu/drm/i810/

drivers/gpu/drm/tinydrm/Kconfig +11 -0

index 16f4b5c91f1b..2c408ac1a900 100644
@@ -10,6 +10,17 @@ menuconfig DRM_TINYDRM
  config TINYDRM_MIPI_DBI
  	tristate
 
+config TINYDRM_HX8357D
+	tristate "DRM support for HX8357D display panels"
+	depends on DRM_TINYDRM && SPI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select TINYDRM_MIPI_DBI
+	help
+	  DRM driver for the following HX8357D panels:
+	  * YX350HV15-T 3.5" 340x350 TFT (Adafruit 3.5")
+
+	  If M is selected the module will be called hx8357d.
+
  config TINYDRM_ILI9225
  	tristate "DRM support for ILI9225 display panels"
  	depends on DRM_TINYDRM && SPI

drivers/gpu/drm/tinydrm/Makefile +1 -0

index 14d99080665a..f823066f7743 100644
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
  obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
 
  # Displays
+obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
  obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
  obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
  obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o

drivers/gpu/drm/tinydrm/hx8357d.c new file +15 -0

new file mode 100644
index 000000000000..51d4da624d57
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for the HX8357D LCD controller
+ *
+ * Copyright 2018 Broadcom
+ * Copyright 2018 David Lechner <[email protected]>
+ * Copyright 2016 Noralf Trønnes
+ * Copyright (C) 2015 Adafruit Industries
+ * Copyright (C) 2013 Christian Vogelgsang
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>

drivers/gpu/drm/tinydrm/hx8357d.c new file +163 -0

+
+#define HX8357D_MADCTL_MY  0x80
+#define HX8357D_MADCTL_MX  0x40
+#define HX8357D_MADCTL_MV  0x20
+#define HX8357D_MADCTL_ML  0x10
+#define HX8357D_MADCTL_RGB 0x00
+#define HX8357D_MADCTL_BGR 0x08
+#define HX8357D_MADCTL_MH  0x04
+
+static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
+			     struct drm_crtc_state *crtc_state,
+			     struct drm_plane_state *plane_state)
+{
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	u8 addr_mode;
+	int ret;
+
+	DRM_DEBUG_KMS("\n");
+
+	ret = mipi_dbi_poweron_conditional_reset(mipi);
+	if (ret < 0)
+		return;
+	if (ret == 1)
+		goto out_enable;
+
+	/* setextc */
+	mipi_dbi_command(mipi, HX8357D_SETC, 0xFF, 0x83, 0x57);
+	msleep(150);
+
+	/* setRGB which also enables SDO */
+	mipi_dbi_command(mipi, HX8357_SETRGB, 0x00, 0x00, 0x06, 0x06);
+
+	/* -1.52V */
+	mipi_dbi_command(mipi, HX8357D_SETCOM, 0x25);
+
+	/* Normal mode 70Hz, Idle mode 55 Hz */
+	mipi_dbi_command(mipi, HX8357_SETOSC, 0x68);
+
+	/* Set Panel - BGR, Gate direction swapped */
+	mipi_dbi_command(mipi, HX8357_SETPANEL, 0x05);
+
+	mipi_dbi_command(mipi, HX8357_SETPWR1,
+			 0x00,  /* Not deep standby */
+			 0x15,  /* BT */
+			 0x1C,  /* VSPR */
+			 0x1C,  /* VSNR */
+			 0x83,  /* AP */
+			 0xAA);  /* FS */
+
+	mipi_dbi_command(mipi, HX8357D_SETSTBA,
+			 0x50,  /* OPON normal */
+			 0x50,  /* OPON idle */
+			 0x01,  /* STBA */
+			 0x3C,  /* STBA */
+			 0x1E,  /* STBA */
+			 0x08);  /* GEN */
+
+	mipi_dbi_command(mipi, HX8357D_SETCYC,
+			 0x02,  /* NW 0x02 */
+			 0x40,  /* RTN */
+			 0x00,  /* DIV */
+			 0x2A,  /* DUM */
+			 0x2A,  /* DUM */
+			 0x0D,  /* GDON */
+			 0x78);  /* GDOFF */
+
+	mipi_dbi_command(mipi, HX8357D_SETGAMMA,
+			 0x02,
+			 0x0A,
+			 0x11,
+			 0x1d,
+			 0x23,
+			 0x35,
+			 0x41,
+			 0x4b,
+			 0x4b,
+			 0x42,
+			 0x3A,
+			 0x27,
+			 0x1B,
+			 0x08,
+			 0x09,
+			 0x03,
+			 0x02,
+			 0x0A,
+			 0x11,
+			 0x1d,
+			 0x23,
+			 0x35,
+			 0x41,
+			 0x4b,
+			 0x4b,
+			 0x42,
+			 0x3A,
+			 0x27,
+			 0x1B,
+			 0x08,
+			 0x09,
+			 0x03,
+			 0x00,
+			 0x01);
+
+	/* 16 bit */
+	mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT,
+			 MIPI_DCS_PIXEL_FMT_16BIT);
+
+	/* TE off */
+	mipi_dbi_command(mipi, MIPI_DCS_SET_TEAR_ON, 0x00);
+
+	/* tear line */
+	mipi_dbi_command(mipi, MIPI_DCS_SET_TEAR_SCANLINE, 0x00, 0x02);
+
+	/* Exit Sleep */
+	mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(150);
+
+	/* display on */
+	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+	usleep_range(5000, 7000);
+
+out_enable:
+	switch (mipi->rotation) {
+	default:
+		addr_mode = HX8357D_MADCTL_MX | HX8357D_MADCTL_MY;
+		break;
+	case 90:
+		addr_mode = HX8357D_MADCTL_MV | HX8357D_MADCTL_MY;
+		break;
+	case 180:
+		addr_mode = 0;
+		break;
+	case 270:
+		addr_mode = HX8357D_MADCTL_MV | HX8357D_MADCTL_MX;
+		break;
+	}
+	mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+	mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+}
+
+static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
+	.enable = yx240qv29_enable,
+	.disable = mipi_dbi_pipe_disable,
+	.update = tinydrm_display_pipe_update,
+	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode yx350hv15_mode = {
+	TINYDRM_MODE(320, 480, 60, 75),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
+
+static struct drm_driver hx8357d_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
+	.fops			= &hx8357d_fops,
+	TINYDRM_GEM_DRIVER_OPS,
+	.debugfs_init		= mipi_dbi_debugfs_init,
+	.name			= "hx8357d",
+	.desc			= "HX8357D",
+	.date			= "20181023",
+	.major			= 1,
+	.minor			= 0,

drivers/gpu/drm/tinydrm/hx8357d.c new file +63 -0

+	{ }
+};
+MODULE_DEVICE_TABLE(spi, hx8357d_id);
+
+static int hx8357d_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dbi *mipi;
+	struct gpio_desc *dc;
+	u32 rotation = 0;
+	int ret;
+
+	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	if (!mipi)
+		return -ENOMEM;
+
+	dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
+		return PTR_ERR(dc);
+	}
+
+	mipi->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(mipi->backlight))
+		return PTR_ERR(mipi->backlight);
+
+	device_property_read_u32(dev, "rotation", &rotation);
+
+	ret = mipi_dbi_spi_init(spi, mipi, dc);
+	if (ret)
+		return ret;
+
+	ret = mipi_dbi_init(&spi->dev, mipi, &hx8357d_pipe_funcs,
+			    &hx8357d_driver, &yx350hv15_mode, rotation);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, mipi);
+
+	return devm_tinydrm_register(&mipi->tinydrm);
+}
+
+static void hx8357d_shutdown(struct spi_device *spi)
+{
+	struct mipi_dbi *mipi = spi_get_drvdata(spi);
+
+	tinydrm_shutdown(&mipi->tinydrm);
+}
+
+static struct spi_driver hx8357d_spi_driver = {
+	.driver = {
+		.name = "hx8357d",
+		.of_match_table = hx8357d_of_match,
+	},
+	.id_table = hx8357d_id,
+	.probe = hx8357d_probe,
+	.shutdown = hx8357d_shutdown,
+};
+module_spi_driver(hx8357d_spi_driver);
+
+MODULE_DESCRIPTION("HX8357D DRM driver");
+MODULE_AUTHOR("Eric Anholt <[email protected]>");
+MODULE_LICENSE("GPL");

drivers/gpu/drm/tinydrm/hx8357d.h new file +61 -0

new file mode 100644
index 000000000000..6180b093f94f
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * This is our library for the Adafruit  ILI9341 Breakout and Shield
+ * ----> http://www.adafruit.com/products/1651
+ *
+ * Check out the links above for our tutorials and wiring diagrams
+ * These displays use SPI to communicate, 4 or 5 pins are required to
+ * interface (RST is optional)
+ * Adafruit invests time and resources providing this open source code,
+ * please support Adafruit and open-source hardware by purchasing
+ * products from Adafruit!
+ *
+ * Written by Limor Fried/Ladyada for Adafruit Industries.
+ * MIT license, all text above must be included in any redistribution
+ */
+
+#ifndef __HX8357_H__
+#define __HX8357_H__
+
+#define HX8357D 0xD
+#define HX8357B 0xB
+
+#define HX8357_TFTWIDTH  320
+#define HX8357_TFTHEIGHT 480
+
+#define HX8357_SETOSC 0xB0
+#define HX8357_SETPWR1 0xB1
+#define HX8357B_SETDISPLAY 0xB2
+#define HX8357_SETRGB 0xB3
+#define HX8357D_SETCOM  0xB6
+
+#define HX8357B_SETDISPMODE  0xB4
+#define HX8357D_SETCYC  0xB4
+#define HX8357B_SETOTP 0xB7
+#define HX8357D_SETC 0xB9
+
+#define HX8357B_SET_PANEL_DRIVING 0xC0
+#define HX8357D_SETSTBA 0xC0
+#define HX8357B_SETDGC  0xC1
+#define HX8357B_SETID  0xC3
+#define HX8357B_SETDDB  0xC4
+#define HX8357B_SETDISPLAYFRAME 0xC5
+#define HX8357B_GAMMASET 0xC8
+#define HX8357B_SETCABC  0xC9
+#define HX8357_SETPANEL  0xCC
+
+#define HX8357B_SETPOWER 0xD0
+#define HX8357B_SETVCOM 0xD1
+#define HX8357B_SETPWRNORMAL 0xD2
+
+#define HX8357B_RDID1   0xDA
+#define HX8357B_RDID2   0xDB
+#define HX8357B_RDID3   0xDC
+#define HX8357B_RDID4   0xDD
+
+#define HX8357D_SETGAMMA 0xE0
+
+#define HX8357B_SETGAMMA 0xC8
+#define HX8357B_SETPANELRELATED  0xE9
+
+/* Color definitions */

Den 24.10.2018 20.43, skrev Eric Anholt:

Quoted 97 lines

I want to sort out support for tinydrm in vc4, so I needed to get a tinydrm-appropriate panel working and this is what I had on hand. This is derived from a combination of ili9341.c from tinydrm and fb_hx8357d.c from staging's fbtft. The register header is copied directly from staging's fbtft, on the assumption that we will delete that copy later.

Signed-off-by Eric Anholt <[email protected]>

---
  MAINTAINERS                       |   7 +
  drivers/gpu/drm/tinydrm/Kconfig   |  11 ++
  drivers/gpu/drm/tinydrm/Makefile  |   1 +
  drivers/gpu/drm/tinydrm/hx8357d.c | 261 ++++++++++++++++++++++++++++++
  drivers/gpu/drm/tinydrm/hx8357d.h |  71 ++++++++
  5 files changed, 351 insertions(+)

create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.c create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.h

MAINTAINERS +7 -0

index 39c3f6682ace..e78971e20a11 100644
@@ -4623,6 +4623,13 @@ S:	Maintained
  F:	drivers/gpu/drm/tinydrm/ili9225.c
  F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR HX8357D PANELS
+M:	Eric Anholt <[email protected]>
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+S:	Maintained
+F:	drivers/gpu/drm/tinydrm/hx8357d.c
+F:	Documentation/devicetree/bindings/display/himax,hx8357d.txt
+
  DRM DRIVER FOR INTEL I810 VIDEO CARDS
  S:	Orphan / Obsolete
  F:	drivers/gpu/drm/i810/

drivers/gpu/drm/tinydrm/Kconfig +11 -0

index 16f4b5c91f1b..2c408ac1a900 100644
@@ -10,6 +10,17 @@ menuconfig DRM_TINYDRM
  config TINYDRM_MIPI_DBI
  	tristate
 
+config TINYDRM_HX8357D
+	tristate "DRM support for HX8357D display panels"
+	depends on DRM_TINYDRM && SPI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select TINYDRM_MIPI_DBI
+	help
+	  DRM driver for the following HX8357D panels:
+	  * YX350HV15-T 3.5" 340x350 TFT (Adafruit 3.5")
+
+	  If M is selected the module will be called hx8357d.
+
  config TINYDRM_ILI9225
  	tristate "DRM support for ILI9225 display panels"
  	depends on DRM_TINYDRM && SPI

drivers/gpu/drm/tinydrm/Makefile +1 -0

index 14d99080665a..f823066f7743 100644
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
  obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
 
  # Displays
+obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
  obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
  obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
  obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o

drivers/gpu/drm/tinydrm/hx8357d.c new file +21 -0

new file mode 100644
index 000000000000..51d4da624d57
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for the HX8357D LCD controller
+ *
+ * Copyright 2018 Broadcom
+ * Copyright 2018 David Lechner <[email protected]>
+ * Copyright 2016 Noralf Trønnes
+ * Copyright (C) 2015 Adafruit Industries
+ * Copyright (C) 2013 Christian Vogelgsang
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modeset_helper.h>

drivers/gpu/drm/tinydrm/hx8357d.c line 25

Quoted 4 lines

drivers/gpu/drm/tinydrm/hx8357d.c new file +4 -0

+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <video/mipi_display.h>
+#include "hx8357d.h"

I prefer to have the defines in the driver instead of an extra header file. The reason is that usually only a handful of defines are actually used, in this case it's 9.

drivers/gpu/drm/tinydrm/hx8357d.c line 198

Quoted 3 lines

drivers/gpu/drm/tinydrm/hx8357d.c new file +3 -0

+
+static const struct spi_device_id hx8357d_id[] = {
+	{ "hx8357d", 0 },

Last time I tried, module autoloading didn't work unless this contains the last part of the compatible. In this case: "yx350hv15". Have you checked that autoloading does work?

Otherwise this looks good:

Reviewed-by Noralf Trønnes <[email protected]>

If the clipped dirty region's x/y happened to align to 256, we would have set the top 8 bits wrong. Noticed by inspection, not by reproducing a bug.

Signed-off-by Eric Anholt <[email protected]>

---

  drivers/gpu/drm/tinydrm/mipi-dbi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

drivers/gpu/drm/tinydrm/mipi-dbi.c +0 -0

index cb3441e51d5f..1bb870021f6e 100644
@@ -240,10 +240,10 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 
  	mipi_dbi_command(mipi, MIPI_DCS_SET_COLUMN_ADDRESS,

I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM.

It seems to work (with DT at
https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get:

https://photos.app.goo.gl/8tuhzPFFoDGamEfk8

If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas?

Quoted 10 lines · Eric Anholt

I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM.

It seems to work (with DT at
https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get:

Quoted 4 lines

https://photos.app.goo.gl/8tuhzPFFoDGamEfk8

If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas?

Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch:

https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro

Now I wonder how we can improve performance of the SPI updates.

Den 25.10.2018 18.29, skrev Eric Anholt:

Quoted 12 lines

Quoted 10 lines · Eric Anholt

I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM.

It seems to work (with DT at
https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get:

Quoted 8 lines

Quoted 4 lines

https://photos.app.goo.gl/8tuhzPFFoDGamEfk8

If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas?

Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch:

https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro

Ah, nice to see this happening!
Getting hw rendering was one of the advantages I saw DRM could provide over fbdev on these displays. Little did I know how complicated graphics was outside fbdev, so I was unable to realise this myself.

The current solution to get hw rendering is to have a userspace process that continously copies the framebuffer:
https://github.com/tasanakorn/rpi-fbcp

It's used by some of the small DIY handheld game consoles that run emulators which requires hw rendering.

Quoted 1 line

Now I wonder how we can improve performance of the SPI updates.

At what SPI speed are you running? The datasheet for most of these display controllers list the max speed as 10MHz, but almost all of them can go faster. Some are reported going as high as 70-80MHz. That's for the pixel data transfer, not the commands. tinydrm/mipi-dbi.c sends commands at 10MHz and pixels at full speed (mipi_dbi_spi_cmd_max_speed()). Most panels I have run at 32MHz or 48MHz.

Almost all the time is spent in the SPI transfer, so every hz counts. On the Pi there's byte swapping because the DMA capable SPI controller can't do 16-bit (tinydrm_swab16()). If I remember correctly this has negligible impact on performance.

The SPI controller/driver on the Pi has some restrictions on the speeds to choose from because the divisor has to be a multiple of two (bcm2835_spi_transfer_one()).

Quoted 33 lines · Noralf Trønnes

Den 25.10.2018 18.29, skrev Eric Anholt:

Quoted 12 lines

Quoted 10 lines · Eric Anholt

I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM.

It seems to work (with DT at
https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get:

Quoted 8 lines

Quoted 4 lines

https://photos.app.goo.gl/8tuhzPFFoDGamEfk8

If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas?

Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch:

https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro

Ah, nice to see this happening!
Getting hw rendering was one of the advantages I saw DRM could provide over fbdev on these displays. Little did I know how complicated graphics was outside fbdev, so I was unable to realise this myself.

The current solution to get hw rendering is to have a userspace process that continously copies the framebuffer:
https://github.com/tasanakorn/rpi-fbcp

It's used by some of the small DIY handheld game consoles that run emulators which requires hw rendering.

Quoted 7 lines

Quoted 1 line

Now I wonder how we can improve performance of the SPI updates.

At what SPI speed are you running? The datasheet for most of these display controllers list the max speed as 10MHz, but almost all of them can go faster. Some are reported going as high as 70-80MHz. That's for the pixel data transfer, not the commands. tinydrm/mipi-dbi.c sends commands at 10MHz and pixels at full speed (mipi_dbi_spi_cmd_max_speed()). Most panels I have run at 32MHz or 48MHz.

I copied the DT from the adafruit tree, which has it at 32mhz. System performance seems to be limited by the copy and format conversion I think -- in particular, I wonder if we shouldn't be doing our dirty copies in our own workqueue. I haven't managed to get any really good profiling data yet, though.

glxgears at 128x128 is nice and smooth, and at 480x320 it's 6fps. That's not filling 32mhz of SPI. On the other hand, I would have expected the uncached reads for the 4-to-2 swapped conversion to be able to go faster than 3.5mb/sec. If it's the uncached reads, we could at least use NEON for the copy to cached, and probably even do the whole conversion in NEON with a bit more thought.

Another option: use a vc4 RCL to do RGBA8888 to RGB565, since that will be less pressure on the bus. But then, I suppose I should just figure out what's going on that makes X11 at RGBA8888 break, and fix that so we don't even have to do that conversion.

I keep hoping there's some way we could feed output from the DISPSLAVE HVS register directly to the SPI master -- FIFO32 gets us close (two 16-bit pixels packed next to each other, leftmost in the lower 2 bytes), but the need for byte swapping (as opposed to R/B swapping) I think makes it impossible.

Quoted 5 lines

Almost all the time is spent in the SPI transfer, so every hz counts. On the Pi there's byte swapping because the DMA capable SPI controller can't do 16-bit (tinydrm_swab16()). If I remember correctly this has negligible impact on performance.

Quoted 3 lines

The SPI controller/driver on the Pi has some restrictions on the speeds to choose from because the divisor has to be a multiple of two (bcm2835_spi_transfer_one()).

That's weird. My specs say CDIV must be a *power* of two, with lower values rounded down. I guess that means we might be running things fast, not slow, though (and at 32mhz out of 250, we should be getting the same CDIV).

Den 26.10.2018 04.30, skrev Eric Anholt:

Quoted 60 lines

Quoted 33 lines · Noralf Trønnes

Den 25.10.2018 18.29, skrev Eric Anholt:

Quoted 12 lines

Quoted 10 lines · Eric Anholt

I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought.  So, last night I ported the fbtft staging driver over to DRM.

It seems to work (with DT at
https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest.  However, when X11 comes up at 16bpp, I get:

Quoted 8 lines

Quoted 4 lines

https://photos.app.goo.gl/8tuhzPFFoDGamEfk8

If I have tinydrm set a preferred bpp of 24, X looks great.  Noralf, any ideas?

Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch:

https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro

Ah, nice to see this happening!
Getting hw rendering was one of the advantages I saw DRM could provide over fbdev on these displays. Little did I know how complicated graphics was outside fbdev, so I was unable to realise this myself.

The current solution to get hw rendering is to have a userspace process that continously copies the framebuffer:
https://github.com/tasanakorn/rpi-fbcp

It's used by some of the small DIY handheld game consoles that run emulators which requires hw rendering.

Quoted 7 lines

Quoted 1 line

Now I wonder how we can improve performance of the SPI updates.

At what SPI speed are you running? The datasheet for most of these display controllers list the max speed as 10MHz, but almost all of them can go faster. Some are reported going as high as 70-80MHz. That's for the pixel data transfer, not the commands. tinydrm/mipi-dbi.c sends commands at 10MHz and pixels at full speed (mipi_dbi_spi_cmd_max_speed()). Most panels I have run at 32MHz or 48MHz.

I copied the DT from the adafruit tree, which has it at 32mhz. System performance seems to be limited by the copy and format conversion I think -- in particular, I wonder if we shouldn't be doing our dirty copies in our own workqueue.  I haven't managed to get any really good profiling data yet, though.

glxgears at 128x128 is nice and smooth, and at 480x320 it's 6fps. That's not filling 32mhz of SPI.  On the other hand, I would have expected the uncached reads for the 4-to-2 swapped conversion to be able to go faster than 3.5mb/sec.  If it's the uncached reads, we could at least use NEON for the copy to cached, and probably even do the whole conversion in NEON with a bit more thought.

Another option: use a vc4 RCL to do RGBA8888 to RGB565, since that will be less pressure on the bus.  But then, I suppose I should just figure out what's going on that makes X11 at RGBA8888 break, and fix that so we don't even have to do that conversion.

Quoted 5 lines

I keep hoping there's some way we could feed output from the DISPSLAVE HVS register directly to the SPI master -- FIFO32 gets us close (two 16-bit pixels packed next to each other, leftmost in the lower 2 bytes), but the need for byte swapping (as opposed to R/B swapping) I think makes it impossible.

I just did some speed tests on a 320x240 display at 3 different speeds. I also tried with byteswapping disabled. Only full updates will benefit from passing the buffer straight through to SPI. This is because partial updates are copied to a transfer buffer anyways to minimize SPI transfer time. No need to transfer things that haven't changed and a memory copy is far cheaper than a SPI transfer.

SPI at 48MHz:

pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency
   48000000

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v
setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30
freq: 24.87Hz
freq: 24.79Hz

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 26.33Hz
freq: 26.30Hz

disable byteswapping:

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 28.40Hz
freq: 28.43Hz

SPI at 64MHz (seems to work):

pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency
   64000000

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v
setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30
freq: 32.74Hz
freq: 32.69Hz

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 35.44Hz
freq: 35.19Hz

disabled byteswapping:

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 39.29Hz
freq: 39.11Hz

SPI at 128MHz (not at all as garbled as I expected):

pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency
  128000000

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v
setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30
freq: 48.69Hz
freq: 48.40Hz

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 53.61Hz
freq: 54.45Hz

disabled byteswapping:

pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 63.16Hz
freq: 64.19Hz

This is how I disabled byteswapping for this test:

drivers/gpu/drm/tinydrm/mipi-dbi.c +2 -0

index cb3441e51d5f..fa5d81521485 100644
@@ -228,6 +228,8 @@ static int mipi_dbi_fb_dirty(struct 
 drm_framebuffer *fb,
         DRM_DEBUG("Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n", fb->base.id,
                   clip.x1, clip.x2, clip.y1, clip.y2);
 
+       full = true;
+       swap = false;

        if (!mipi->dc || !full || swap || fb->format->format == DRM_FORMAT_XRGB8888) { tr = mipi->tx_buf;

Quoted 5 lines

Quoted 5 lines

Almost all the time is spent in the SPI transfer, so every hz counts. On the Pi there's byte swapping because the DMA capable SPI controller can't do 16-bit (tinydrm_swab16()). If I remember correctly this has negligible impact on performance.

Quoted 7 lines

Quoted 3 lines

The SPI controller/driver on the Pi has some restrictions on the speeds to choose from because the divisor has to be a multiple of two (bcm2835_spi_transfer_one()).

That's weird.  My specs say CDIV must be a *power* of two, with lower values rounded down.  I guess that means we might be running things fast, not slow, though (and at 32mhz out of 250, we should be getting the same CDIV).

Yes, that's what the datasheet says.
When fbtft was out-of-tree I distributed a custom kernel with fbtft and Martin Sperl's DMA capable spi-bcm2708. In that version I also allowed odd cdiv's with no ill effects reported:
https://github.com/notro/spi-bcm2708/wiki#spi-clock-divider
(see the link to the forum post for details)
Maybe the hw just ignores odd cdiv's, I don't know.

Quoted 12 lines · Eric Anholt

I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM.

It seems to work (with DT at
https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get:

https://photos.app.goo.gl/8tuhzPFFoDGamEfk8

Quoted 2 lines

If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas?

Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch:

https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro

Now I wonder how we can improve performance of the SPI updates.