[PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[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

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

Eric Anholt (3):
  dt-bindings: new binding for Himax HX8357D display panels
  drm: Add an hx8367d tinydrm driver.
  drm/tinydrm: Fix setting of the column/page end addresses.

 .../bindings/display/himax,hx8357d.txt        |  25 ++
 drivers/gpu/drm/tinydrm/Kconfig               |  11 +
 drivers/gpu/drm/tinydrm/Makefile              |   1 +
 drivers/gpu/drm/tinydrm/hx8357d.c             | 262 ++++++++++++++++++
 drivers/gpu/drm/tinydrm/hx8357d.h             |  71 +++++
 drivers/gpu/drm/tinydrm/mipi-dbi.c            |   4 +-
 6 files changed, 372 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/himax,hx8357d.txt
 create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.c
 create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.h

-- 
2.19.1

[PATCH 1/3] dt-bindings: new binding for Himax HX8357D display panels

[Eric Anholt]

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 <eric@anholt.net>
---
 .../bindings/display/himax,hx8357d.txt        | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/himax,hx8357d.txt

diff --git a/Documentation/devicetree/bindings/display/himax,hx8357d.txt b/Documentation/devicetree/bindings/display/himax,hx8357d.txt
new file mode 100644
index 000000000000..48586e570be0
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/himax,hx8357d.txt
@@ -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
+
+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>;
+	};
-- 
2.19.1

[PATCH 2/3] drm: Add an hx8367d tinydrm driver.

[Eric Anholt]

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 <eric@anholt.net>
---
 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

diff --git a/MAINTAINERS b/MAINTAINERS
index 39c3f6682ace..e78971e20a11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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 <eric@anholt.net>
+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/
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 16f4b5c91f1b..2c408ac1a900 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -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
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 14d99080665a..f823066f7743 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -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
diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
new file mode 100644
index 000000000000..51d4da624d57
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for the HX8357D LCD controller
+ *
+ * Copyright 2018 Broadcom
+ * Copyright 2018 David Lechner <david@lechnology.com>
+ * 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>
+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <video/mipi_display.h>
+#include "hx8357d.h"
+
+#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,
+};
+
+static const struct of_device_id hx8357d_of_match[] = {
+	{ .compatible = "adafruit,yx350hv15" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, hx8357d_of_match);
+
+static const struct spi_device_id hx8357d_id[] = {
+	{ "hx8357d", 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 <eric@anholt.net>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tinydrm/hx8357d.h b/drivers/gpu/drm/tinydrm/hx8357d.h
new file mode 100644
index 000000000000..6180b093f94f
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/hx8357d.h
@@ -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 */
+#define	HX8357_BLACK   0x0000
+#define	HX8357_BLUE    0x001F
+#define	HX8357_RED     0xF800
+#define	HX8357_GREEN   0x07E0
+#define HX8357_CYAN    0x07FF
+#define HX8357_MAGENTA 0xF81F
+#define HX8357_YELLOW  0xFFE0
+#define HX8357_WHITE   0xFFFF
+
+#endif /* __HX8357_H__ */
-- 
2.19.1

[PATCH 3/3] drm/tinydrm: Fix setting of the column/page end addresses.

[Eric Anholt]

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 <eric@anholt.net>
---
 drivers/gpu/drm/tinydrm/mipi-dbi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index cb3441e51d5f..1bb870021f6e 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -240,10 +240,10 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 
 	mipi_dbi_command(mipi, MIPI_DCS_SET_COLUMN_ADDRESS,
 			 (clip.x1 >> 8) & 0xFF, clip.x1 & 0xFF,
-			 (clip.x2 >> 8) & 0xFF, (clip.x2 - 1) & 0xFF);
+			 ((clip.x2 - 1) >> 8) & 0xFF, (clip.x2 - 1) & 0xFF);
 	mipi_dbi_command(mipi, MIPI_DCS_SET_PAGE_ADDRESS,
 			 (clip.y1 >> 8) & 0xFF, clip.y1 & 0xFF,
-			 (clip.y2 >> 8) & 0xFF, (clip.y2 - 1) & 0xFF);
+			 ((clip.y2 - 1) >> 8) & 0xFF, (clip.y2 - 1) & 0xFF);
 
 	ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, tr,
 				(clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
-- 
2.19.1

Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[Eric Anholt]

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

Eric Anholt <eric@anholt.net> writes:

> 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?

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.

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

Re: [PATCH 1/3] dt-bindings: new binding for Himax HX8357D display panels

[Rob Herring]

On Wed, Oct 24, 2018 at 11:43:11AM -0700, Eric Anholt wrote:
> 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 <eric@anholt.net>
> ---
>  .../bindings/display/himax,hx8357d.txt        | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/himax,hx8357d.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/himax,hx8357d.txt b/Documentation/devicetree/bindings/display/himax,hx8357d.txt
> new file mode 100644
> index 000000000000..48586e570be0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/himax,hx8357d.txt
> @@ -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

You forgot reg. Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

> +
> +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>;
> +	};
> -- 
> 2.19.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[Noralf Trønnes]

Den 25.10.2018 18.29, skrev Eric Anholt:
> Eric Anholt <eric@anholt.net> writes:
>
>> 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?
> 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.

> 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()).

A full update on a 320x480 RGB565 panel is 262.5kB, so it's a lot to push
over SPI. A 2.8" 320x240 panel is more suitable for video fps, because of
the lower resolution.

I'll look at the patches during the weekend.

Noralf.

Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[Eric Anholt]

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

Noralf Trønnes <noralf@tronnes.org> writes:

> Den 25.10.2018 18.29, skrev Eric Anholt:
>> Eric Anholt <eric@anholt.net> writes:
>>
>>> 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?
>> 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.
>
>> 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.

> 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()).

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).

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

Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[Noralf Trønnes]

Den 26.10.2018 04.30, skrev Eric Anholt:
> Noralf Trønnes <noralf@tronnes.org> writes:
>
>> Den 25.10.2018 18.29, skrev Eric Anholt:
>>> Eric Anholt <eric@anholt.net> writes:
>>>
>>>> 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?
>>> 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.
>>
>>> 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.

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:

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index cb3441e51d5f..fa5d81521485 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -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;


>> 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()).
> 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.

Noralf.

Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[Noralf Trønnes]

Den 26.10.2018 21.16, skrev Noralf Trønnes:
>
> Den 26.10.2018 04.30, skrev Eric Anholt:
>> Noralf Trønnes <noralf@tronnes.org> writes:
>>
>>> Den 25.10.2018 18.29, skrev Eric Anholt:
>>>> Eric Anholt <eric@anholt.net> writes:
>>>>
>>>>> 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?
>>>> 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.
>>>
>>>> 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.
>
> 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:
>
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
> b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index cb3441e51d5f..fa5d81521485 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -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;
>
>
>>> 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()).
>> 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.
>

I lifted the cdiv being odd limitation but it didn't give more speeds.
The test does demonstrate that it doesn't have to be a power of two though.

pi@pi2835:~$ od -An -vtu4 --endian=big 
/sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency
    40000000
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.32Hz
freq: 26.22Hz

pi@pi2835:~$ dmesg | grep cdiv
[   59.820514] bcm2835_spi_transfer_one: cdiv=7


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@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 26.32Hz
freq: 26.28Hz

pi@pi2835:~$ dmesg | grep cdiv
[   59.250549] bcm2835_spi_transfer_one: cdiv=6


pi@pi2835:~$ od -An -vtu4 --endian=big 
/sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency
    56000000
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.33Hz
freq: 35.42Hz

pi@pi2835:~$ dmesg | grep cdiv
[   67.760747] bcm2835_spi_transfer_one: cdiv=5


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@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 35.40Hz
freq: 35.35Hz

pi@pi2835:~$ dmesg | grep cdiv
[   76.061747] bcm2835_spi_transfer_one: cdiv=4


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

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

pi@pi2835:~$ dmesg | grep cdiv
[  623.200407] bcm2835_spi_transfer_one: cdiv=3


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@RG16 -v
setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30
freq: 54.43Hz
freq: 54.31Hz

pi@pi2835:~$ dmesg | grep cdiv
[   65.350713] bcm2835_spi_transfer_one: cdiv=2


I can add here that XRGB8888 was added to support things like plymouth
that only supports that one format. RGB565 is the native format supported
by the driver. These MIPI compatible controllers do support RGB888, but
there's no point in implementing it since it will increase the transfer
time by 50% due to the extra byte, so it's of little use.

Noralf.

Re: [PATCH 1/3] dt-bindings: new binding for Himax HX8357D display panels

[Noralf Trønnes]

Den 24.10.2018 20.43, skrev Eric Anholt:
> 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.

Typo s/hx8367d/hx8357d/

The adafruit display has an AXP803 that resets the controller.

Acked-by: Noralf Trønnes <noralf@tronnes.org>

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>   .../bindings/display/himax,hx8357d.txt        | 25 +++++++++++++++++++
>   1 file changed, 25 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/himax,hx8357d.txt
>
> diff --git a/Documentation/devicetree/bindings/display/himax,hx8357d.txt b/Documentation/devicetree/bindings/display/himax,hx8357d.txt
> new file mode 100644
> index 000000000000..48586e570be0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/himax,hx8357d.txt
> @@ -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
> +
> +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>;
> +	};

Re: [PATCH 2/3] drm: Add an hx8367d tinydrm driver.

[Noralf Trønnes]

Den 24.10.2018 20.43, skrev Eric Anholt:
> 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 <eric@anholt.net>
> ---
>   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
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 39c3f6682ace..e78971e20a11 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -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 <eric@anholt.net>
> +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/
> diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
> index 16f4b5c91f1b..2c408ac1a900 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -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
> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
> index 14d99080665a..f823066f7743 100644
> --- a/drivers/gpu/drm/tinydrm/Makefile
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -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
> diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
> new file mode 100644
> index 000000000000..51d4da624d57
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/hx8357d.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DRM driver for the HX8357D LCD controller
> + *
> + * Copyright 2018 Broadcom
> + * Copyright 2018 David Lechner <david@lechnology.com>
> + * 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>
> +#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.

> +
> +#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,
> +};
> +
> +static const struct of_device_id hx8357d_of_match[] = {
> +	{ .compatible = "adafruit,yx350hv15" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, hx8357d_of_match);
> +
> +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 <noralf@tronnes.org>

> +	{ }
> +};
> +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 <eric@anholt.net>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/tinydrm/hx8357d.h b/drivers/gpu/drm/tinydrm/hx8357d.h
> new file mode 100644
> index 000000000000..6180b093f94f
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/hx8357d.h
> @@ -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 */
> +#define	HX8357_BLACK   0x0000
> +#define	HX8357_BLUE    0x001F
> +#define	HX8357_RED     0xF800
> +#define	HX8357_GREEN   0x07E0
> +#define HX8357_CYAN    0x07FF
> +#define HX8357_MAGENTA 0xF81F
> +#define HX8357_YELLOW  0xFFE0
> +#define HX8357_WHITE   0xFFFF
> +
> +#endif /* __HX8357_H__ */

Re: [PATCH 3/3] drm/tinydrm: Fix setting of the column/page end addresses.

[Noralf Trønnes]

Den 24.10.2018 20.43, skrev Eric Anholt:
> 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 <eric@anholt.net>
> ---

Good catch.
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

>   drivers/gpu/drm/tinydrm/mipi-dbi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index cb3441e51d5f..1bb870021f6e 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -240,10 +240,10 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
>   
>   	mipi_dbi_command(mipi, MIPI_DCS_SET_COLUMN_ADDRESS,
>   			 (clip.x1 >> 8) & 0xFF, clip.x1 & 0xFF,
> -			 (clip.x2 >> 8) & 0xFF, (clip.x2 - 1) & 0xFF);
> +			 ((clip.x2 - 1) >> 8) & 0xFF, (clip.x2 - 1) & 0xFF);
>   	mipi_dbi_command(mipi, MIPI_DCS_SET_PAGE_ADDRESS,
>   			 (clip.y1 >> 8) & 0xFF, clip.y1 & 0xFF,
> -			 (clip.y2 >> 8) & 0xFF, (clip.y2 - 1) & 0xFF);
> +			 ((clip.y2 - 1) >> 8) & 0xFF, (clip.y2 - 1) & 0xFF);
>   
>   	ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, tr,
>   				(clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);

Re: [PATCH 2/3] drm: Add an hx8367d tinydrm driver.

[Eric Anholt]

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

Noralf Trønnes <noralf@tronnes.org> writes:

> Den 24.10.2018 20.43, skrev Eric Anholt:
>> 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 <eric@anholt.net>
>> ---
>>   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
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 39c3f6682ace..e78971e20a11 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -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 <eric@anholt.net>
>> +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/
>> diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
>> index 16f4b5c91f1b..2c408ac1a900 100644
>> --- a/drivers/gpu/drm/tinydrm/Kconfig
>> +++ b/drivers/gpu/drm/tinydrm/Kconfig
>> @@ -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
>> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
>> index 14d99080665a..f823066f7743 100644
>> --- a/drivers/gpu/drm/tinydrm/Makefile
>> +++ b/drivers/gpu/drm/tinydrm/Makefile
>> @@ -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
>> diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
>> new file mode 100644
>> index 000000000000..51d4da624d57
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tinydrm/hx8357d.c
>> @@ -0,0 +1,261 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * DRM driver for the HX8357D LCD controller
>> + *
>> + * Copyright 2018 Broadcom
>> + * Copyright 2018 David Lechner <david@lechnology.com>
>> + * 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>
>> +#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.

Yeah.  Also, that license on the header is pretty sketchy.  I've written
my own defines based on reading the HX8357D spec and included it in the
driver instead.

>> +
>> +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?

I hadn't since I changed the compatible string to adafruit.  Thanks!

> Otherwise this looks good:
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

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

Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen

[Noralf Trønnes]

Den 25.10.2018 18.29, skrev Eric Anholt:
> Eric Anholt <eric@anholt.net> writes:
>
>> 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?
> 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.

I just remembered that tinydrm does a full flush on page flips. And if
the dirtyfb ioctl is also used you end up with two flushes. This could
explain bad performance. I have been waiting for the dirtyfb through
atomic work to come to fruition, but nothing merged yet. That would give
dirty tracking on page flips.

Noralf.