All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/tinydrm: new dirver for ILI9341 displays
@ 2018-05-15  1:43 ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	limor, Nitin Patil, linux-kernel

This series adds a new tinydrm driver for the Ilitek ILI9341 controller and
a 2.4" display panel that uses this controller.

A few things to note here:
* The datasheet for this display[1] doesn't have a vendor mentioned on it
  anywhere, so I have used "noname" as the vendor prefix. If someone has a
  better suggestion, please speak up.
* The driver is basically a copy of mi0283qt.c with a new init sequence,
  a different physical panel size, fixed (as in corrected) rotation handling,
  and dropped PM (since I don't have a way to test it). Do we want to try to
  share code with these two drivers (it's not much)?
* The MAINTAINERS patch for ili9225 is included so we don't end up with a merge
  conflict later on.

[1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf

David Lechner (3):
  MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
  dt-bindings: new binding for Ilitek ILI9341 display panels
  drm/tinydrm: new driver for ILI9341 display panels

 .../bindings/display/ilitek,ili9341.txt       |  27 ++
 MAINTAINERS                                   |   8 +-
 drivers/gpu/drm/tinydrm/Kconfig               |  10 +
 drivers/gpu/drm/tinydrm/Makefile              |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c             | 239 ++++++++++++++++++
 5 files changed, 284 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

-- 
2.17.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/3] drm/tinydrm: new dirver for ILI9341 displays
@ 2018-05-15  1:43 ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, limor, David Lechner, linux-kernel, Rob Herring,
	Nitin Patil

This series adds a new tinydrm driver for the Ilitek ILI9341 controller and
a 2.4" display panel that uses this controller.

A few things to note here:
* The datasheet for this display[1] doesn't have a vendor mentioned on it
  anywhere, so I have used "noname" as the vendor prefix. If someone has a
  better suggestion, please speak up.
* The driver is basically a copy of mi0283qt.c with a new init sequence,
  a different physical panel size, fixed (as in corrected) rotation handling,
  and dropped PM (since I don't have a way to test it). Do we want to try to
  share code with these two drivers (it's not much)?
* The MAINTAINERS patch for ili9225 is included so we don't end up with a merge
  conflict later on.

[1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf

David Lechner (3):
  MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
  dt-bindings: new binding for Ilitek ILI9341 display panels
  drm/tinydrm: new driver for ILI9341 display panels

 .../bindings/display/ilitek,ili9341.txt       |  27 ++
 MAINTAINERS                                   |   8 +-
 drivers/gpu/drm/tinydrm/Kconfig               |  10 +
 drivers/gpu/drm/tinydrm/Makefile              |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c             | 239 ++++++++++++++++++
 5 files changed, 284 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

-- 
2.17.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
  2018-05-15  1:43 ` David Lechner
@ 2018-05-15  1:43   ` David Lechner
  -1 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	limor, Nitin Patil, linux-kernel

This fixes the path to the ilitek,ili9225 device tree binding file.

Signed-off-by: David Lechner <david@lechnology.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 334a00350922..bc219de9cbee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4478,7 +4478,7 @@ DRM DRIVER FOR ILITEK ILI9225 PANELS
 M:	David Lechner <david@lechnology.com>
 S:	Maintained
 F:	drivers/gpu/drm/tinydrm/ili9225.c
-F:	Documentation/devicetree/bindings/display/ili9225.txt
+F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
 DRM DRIVER FOR INTEL I810 VIDEO CARDS
 S:	Orphan / Obsolete
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/3] MAINTAINERS: fix path to ilitek, ili9225 device tree bindings
@ 2018-05-15  1:43   ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, limor, David Lechner, linux-kernel, Rob Herring,
	Nitin Patil

This fixes the path to the ilitek,ili9225 device tree binding file.

Signed-off-by: David Lechner <david@lechnology.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 334a00350922..bc219de9cbee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4478,7 +4478,7 @@ DRM DRIVER FOR ILITEK ILI9225 PANELS
 M:	David Lechner <david@lechnology.com>
 S:	Maintained
 F:	drivers/gpu/drm/tinydrm/ili9225.c
-F:	Documentation/devicetree/bindings/display/ili9225.txt
+F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
 DRM DRIVER FOR INTEL I810 VIDEO CARDS
 S:	Orphan / Obsolete
-- 
2.17.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels
  2018-05-15  1:43 ` David Lechner
@ 2018-05-15  1:43   ` David Lechner
  -1 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	limor, Nitin Patil, linux-kernel

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

The vendor prefix "noname" is used because the vendor is not known.
The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
board[2] and in Mindsensors' PiStorms[3].

[1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
[2]: https://www.adafruit.com/product/2478
[3]: http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot

Signed-off-by: David Lechner <david@lechnology.com>
---
 .../bindings/display/ilitek,ili9341.txt       | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
new file mode 100644
index 000000000000..0fc90b2dd732
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
@@ -0,0 +1,27 @@
+Ilitek ILI9341 display panels
+
+This binding is for display panels using an Ilitek ILI9341 controller in SPI
+mode.
+
+Required properties:
+- compatible:	"noname,yx240qv29", "ilitek,ili9341"
+- dc-gpios:	D/C pin
+- reset-gpios:	Reset 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 = "noname,yx240qv29", "ilitek,ili9341";
+		reg = <0>;
+		spi-max-frequency = <32000000>;
+		dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+		rotation = <270>;
+		backlight = <&backlight>;
+	};
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels
@ 2018-05-15  1:43   ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, limor, David Lechner, linux-kernel, Rob Herring,
	Nitin Patil

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

The vendor prefix "noname" is used because the vendor is not known.
The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
board[2] and in Mindsensors' PiStorms[3].

[1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
[2]: https://www.adafruit.com/product/2478
[3]: http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot

Signed-off-by: David Lechner <david@lechnology.com>
---
 .../bindings/display/ilitek,ili9341.txt       | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
new file mode 100644
index 000000000000..0fc90b2dd732
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
@@ -0,0 +1,27 @@
+Ilitek ILI9341 display panels
+
+This binding is for display panels using an Ilitek ILI9341 controller in SPI
+mode.
+
+Required properties:
+- compatible:	"noname,yx240qv29", "ilitek,ili9341"
+- dc-gpios:	D/C pin
+- reset-gpios:	Reset 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 = "noname,yx240qv29", "ilitek,ili9341";
+		reg = <0>;
+		spi-max-frequency = <32000000>;
+		dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+		rotation = <270>;
+		backlight = <&backlight>;
+	};
-- 
2.17.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
  2018-05-15  1:43 ` David Lechner
@ 2018-05-15  1:43   ` David Lechner
  -1 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	limor, Nitin Patil, linux-kernel

This adds a new driver for display panels that use the Ilitek ILI9341
controller. It currently supports a single display panel, namely
the YX240QV29-T (e.g. Adafruit 2.4" TFT).

The init sequence is from the Adafruit Python library for the ILI9341
controller. https://github.com/adafruit/Adafruit_Python_ILI9341

Signed-off-by: David Lechner <david@lechnology.com>
---
 MAINTAINERS                       |   6 +
 drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
 drivers/gpu/drm/tinydrm/Makefile  |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bc219de9cbee..ffa099abbd79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4480,6 +4480,12 @@ S:	Maintained
 F:	drivers/gpu/drm/tinydrm/ili9225.c
 F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M:	David Lechner <david@lechnology.com>
+S:	Maintained
+F:	drivers/gpu/drm/tinydrm/ili9341.c
+F:	Documentation/devicetree/bindings/display/ilitek,ili9341.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 4592a5e3f20b..7a8008b0783f 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -20,6 +20,16 @@ config TINYDRM_ILI9225
 
 	  If M is selected the module will be called ili9225.
 
+config TINYDRM_ILI9341
+	tristate "DRM support for ILI9341 display panels"
+	depends on DRM_TINYDRM && SPI
+	select TINYDRM_MIPI_DBI
+	help
+	  DRM driver for the following Ilitek ILI9341 panels:
+	  * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
+
+	  If M is selected the module will be called ili9341.
+
 config TINYDRM_MI0283QT
 	tristate "DRM support for MI0283QT"
 	depends on DRM_TINYDRM && SPI
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 49a111929724..14d99080665a 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
 
 # Displays
 obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
+obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
 obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
 obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
new file mode 100644
index 000000000000..2ce4244a68c3
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <david@lechnology.com>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_modeset_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <video/mipi_display.h>
+
+#define ILI9341_FRMCTR1		0xb1
+#define ILI9341_DISCTRL		0xb6
+#define ILI9341_ETMOD		0xb7
+
+#define ILI9341_PWCTRL1		0xc0
+#define ILI9341_PWCTRL2		0xc1
+#define ILI9341_VMCTRL1		0xc5
+#define ILI9341_VMCTRL2		0xc7
+#define ILI9341_PWCTRLA		0xcb
+#define ILI9341_PWCTRLB		0xcf
+
+#define ILI9341_PGAMCTRL	0xe0
+#define ILI9341_NGAMCTRL	0xe1
+#define ILI9341_DTCTRLA		0xe8
+#define ILI9341_DTCTRLB		0xea
+#define ILI9341_PWRSEQ		0xed
+
+#define ILI9341_EN3GAM		0xf2
+#define ILI9341_PUMPCTRL	0xf7
+
+#define ILI9341_MADCTL_BGR	BIT(3)
+#define ILI9341_MADCTL_MV	BIT(5)
+#define ILI9341_MADCTL_MX	BIT(6)
+#define ILI9341_MADCTL_MY	BIT(7)
+
+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;
+
+	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
+
+	mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+	mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+	mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+	mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+	mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
+	mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
+
+	/* Power Control */
+	mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
+	mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
+	/* VCOM */
+	mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
+	mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
+
+	/* Memory Access Control */
+	mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
+
+	/* Frame Rate */
+	mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
+
+	/* Gamma */
+	mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
+	mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
+			 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+			 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+	mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
+			 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+			 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+	/* DDRAM */
+	mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
+
+	/* Display */
+	mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
+	mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(100);
+
+	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+	msleep(100);
+
+out_enable:
+	switch (mipi->rotation) {
+	default:
+		addr_mode = ILI9341_MADCTL_MX;
+		break;
+	case 90:
+		addr_mode = ILI9341_MADCTL_MV;
+		break;
+	case 180:
+		addr_mode = ILI9341_MADCTL_MY;
+		break;
+	case 270:
+		addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
+			    ILI9341_MADCTL_MX;
+		break;
+	}
+	addr_mode |= ILI9341_MADCTL_BGR;
+	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 ili9341_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 yx240qv29_mode = {
+	TINYDRM_MODE(240, 320, 37, 49),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
+
+static struct drm_driver ili9341_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
+				  DRIVER_ATOMIC,
+	.fops			= &ili9341_fops,
+	TINYDRM_GEM_DRIVER_OPS,
+	.lastclose		= drm_fb_helper_lastclose,
+	.debugfs_init		= mipi_dbi_debugfs_init,
+	.name			= "ili9341",
+	.desc			= "Ilitek ILI9341",
+	.date			= "20180514",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static const struct of_device_id ili9341_of_match[] = {
+	{ .compatible = "noname,yx240qv29" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ili9341_of_match);
+
+static const struct spi_device_id ili9341_id[] = {
+	{ "yx240qv29", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, ili9341_id);
+
+static int ili9341_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;
+
+	mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(mipi->reset)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
+		return PTR_ERR(mipi->reset);
+	}
+
+	dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
+		return PTR_ERR(dc);
+	}
+
+	mipi->regulator = devm_regulator_get(dev, "power");
+	if (IS_ERR(mipi->regulator))
+		return PTR_ERR(mipi->regulator);
+
+	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, &ili9341_pipe_funcs,
+			    &ili9341_driver, &yx240qv29_mode, rotation);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, mipi);
+
+	return devm_tinydrm_register(&mipi->tinydrm);
+}
+
+static void ili9341_shutdown(struct spi_device *spi)
+{
+	struct mipi_dbi *mipi = spi_get_drvdata(spi);
+
+	tinydrm_shutdown(&mipi->tinydrm);
+}
+
+static struct spi_driver ili9341_spi_driver = {
+	.driver = {
+		.name = "ili9341",
+		.of_match_table = ili9341_of_match,
+	},
+	.id_table = ili9341_id,
+	.probe = ili9341_probe,
+	.shutdown = ili9341_shutdown,
+};
+module_spi_driver(ili9341_spi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_LICENSE("GPL");
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
@ 2018-05-15  1:43   ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-15  1:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, limor, David Lechner, linux-kernel, Rob Herring,
	Nitin Patil

This adds a new driver for display panels that use the Ilitek ILI9341
controller. It currently supports a single display panel, namely
the YX240QV29-T (e.g. Adafruit 2.4" TFT).

The init sequence is from the Adafruit Python library for the ILI9341
controller. https://github.com/adafruit/Adafruit_Python_ILI9341

Signed-off-by: David Lechner <david@lechnology.com>
---
 MAINTAINERS                       |   6 +
 drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
 drivers/gpu/drm/tinydrm/Makefile  |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bc219de9cbee..ffa099abbd79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4480,6 +4480,12 @@ S:	Maintained
 F:	drivers/gpu/drm/tinydrm/ili9225.c
 F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M:	David Lechner <david@lechnology.com>
+S:	Maintained
+F:	drivers/gpu/drm/tinydrm/ili9341.c
+F:	Documentation/devicetree/bindings/display/ilitek,ili9341.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 4592a5e3f20b..7a8008b0783f 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -20,6 +20,16 @@ config TINYDRM_ILI9225
 
 	  If M is selected the module will be called ili9225.
 
+config TINYDRM_ILI9341
+	tristate "DRM support for ILI9341 display panels"
+	depends on DRM_TINYDRM && SPI
+	select TINYDRM_MIPI_DBI
+	help
+	  DRM driver for the following Ilitek ILI9341 panels:
+	  * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
+
+	  If M is selected the module will be called ili9341.
+
 config TINYDRM_MI0283QT
 	tristate "DRM support for MI0283QT"
 	depends on DRM_TINYDRM && SPI
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 49a111929724..14d99080665a 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
 
 # Displays
 obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
+obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
 obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
 obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
new file mode 100644
index 000000000000..2ce4244a68c3
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <david@lechnology.com>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_modeset_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <video/mipi_display.h>
+
+#define ILI9341_FRMCTR1		0xb1
+#define ILI9341_DISCTRL		0xb6
+#define ILI9341_ETMOD		0xb7
+
+#define ILI9341_PWCTRL1		0xc0
+#define ILI9341_PWCTRL2		0xc1
+#define ILI9341_VMCTRL1		0xc5
+#define ILI9341_VMCTRL2		0xc7
+#define ILI9341_PWCTRLA		0xcb
+#define ILI9341_PWCTRLB		0xcf
+
+#define ILI9341_PGAMCTRL	0xe0
+#define ILI9341_NGAMCTRL	0xe1
+#define ILI9341_DTCTRLA		0xe8
+#define ILI9341_DTCTRLB		0xea
+#define ILI9341_PWRSEQ		0xed
+
+#define ILI9341_EN3GAM		0xf2
+#define ILI9341_PUMPCTRL	0xf7
+
+#define ILI9341_MADCTL_BGR	BIT(3)
+#define ILI9341_MADCTL_MV	BIT(5)
+#define ILI9341_MADCTL_MX	BIT(6)
+#define ILI9341_MADCTL_MY	BIT(7)
+
+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;
+
+	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
+
+	mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+	mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+	mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+	mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+	mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
+	mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
+
+	/* Power Control */
+	mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
+	mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
+	/* VCOM */
+	mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
+	mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
+
+	/* Memory Access Control */
+	mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
+
+	/* Frame Rate */
+	mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
+
+	/* Gamma */
+	mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
+	mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
+			 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+			 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+	mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
+			 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+			 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+	/* DDRAM */
+	mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
+
+	/* Display */
+	mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
+	mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(100);
+
+	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+	msleep(100);
+
+out_enable:
+	switch (mipi->rotation) {
+	default:
+		addr_mode = ILI9341_MADCTL_MX;
+		break;
+	case 90:
+		addr_mode = ILI9341_MADCTL_MV;
+		break;
+	case 180:
+		addr_mode = ILI9341_MADCTL_MY;
+		break;
+	case 270:
+		addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
+			    ILI9341_MADCTL_MX;
+		break;
+	}
+	addr_mode |= ILI9341_MADCTL_BGR;
+	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 ili9341_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 yx240qv29_mode = {
+	TINYDRM_MODE(240, 320, 37, 49),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
+
+static struct drm_driver ili9341_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
+				  DRIVER_ATOMIC,
+	.fops			= &ili9341_fops,
+	TINYDRM_GEM_DRIVER_OPS,
+	.lastclose		= drm_fb_helper_lastclose,
+	.debugfs_init		= mipi_dbi_debugfs_init,
+	.name			= "ili9341",
+	.desc			= "Ilitek ILI9341",
+	.date			= "20180514",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static const struct of_device_id ili9341_of_match[] = {
+	{ .compatible = "noname,yx240qv29" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ili9341_of_match);
+
+static const struct spi_device_id ili9341_id[] = {
+	{ "yx240qv29", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, ili9341_id);
+
+static int ili9341_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;
+
+	mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(mipi->reset)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
+		return PTR_ERR(mipi->reset);
+	}
+
+	dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
+		return PTR_ERR(dc);
+	}
+
+	mipi->regulator = devm_regulator_get(dev, "power");
+	if (IS_ERR(mipi->regulator))
+		return PTR_ERR(mipi->regulator);
+
+	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, &ili9341_pipe_funcs,
+			    &ili9341_driver, &yx240qv29_mode, rotation);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, mipi);
+
+	return devm_tinydrm_register(&mipi->tinydrm);
+}
+
+static void ili9341_shutdown(struct spi_device *spi)
+{
+	struct mipi_dbi *mipi = spi_get_drvdata(spi);
+
+	tinydrm_shutdown(&mipi->tinydrm);
+}
+
+static struct spi_driver ili9341_spi_driver = {
+	.driver = {
+		.name = "ili9341",
+		.of_match_table = ili9341_of_match,
+	},
+	.id_table = ili9341_id,
+	.probe = ili9341_probe,
+	.shutdown = ili9341_shutdown,
+};
+module_spi_driver(ili9341_spi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_LICENSE("GPL");
-- 
2.17.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
  2018-05-15  1:43   ` David Lechner
@ 2018-05-15 21:43     ` Andy Shevchenko
  -1 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2018-05-15 21:43 UTC (permalink / raw)
  To: David Lechner
  Cc: dri-devel, devicetree, Noralf Trønnes, Rob Herring,
	Mark Rutland, limor, Nitin Patil, Linux Kernel Mailing List

On Tue, May 15, 2018 at 4:43 AM, David Lechner <david@lechnology.com> wrote:
> This adds a new driver for display panels that use the Ilitek ILI9341
> controller. It currently supports a single display panel, namely
> the YX240QV29-T (e.g. Adafruit 2.4" TFT).
>
> The init sequence is from the Adafruit Python library for the ILI9341
> controller. https://github.com/adafruit/Adafruit_Python_ILI9341

Some minor style nitpicks, otherwise LGTM

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  MAINTAINERS                       |   6 +
>  drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
>  drivers/gpu/drm/tinydrm/Makefile  |   1 +
>  drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
>  4 files changed, 256 insertions(+)
>  create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc219de9cbee..ffa099abbd79 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4480,6 +4480,12 @@ S:       Maintained
>  F:     drivers/gpu/drm/tinydrm/ili9225.c
>  F:     Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>
> +DRM DRIVER FOR ILITEK ILI9341 PANELS
> +M:     David Lechner <david@lechnology.com>
> +S:     Maintained
> +F:     drivers/gpu/drm/tinydrm/ili9341.c
> +F:     Documentation/devicetree/bindings/display/ilitek,ili9341.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 4592a5e3f20b..7a8008b0783f 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -20,6 +20,16 @@ config TINYDRM_ILI9225
>
>           If M is selected the module will be called ili9225.
>
> +config TINYDRM_ILI9341
> +       tristate "DRM support for ILI9341 display panels"

> +       depends on DRM_TINYDRM && SPI

Can't we do something like

if SPI

...

endif

?

> +       select TINYDRM_MIPI_DBI
> +       help
> +         DRM driver for the following Ilitek ILI9341 panels:
> +         * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
> +
> +         If M is selected the module will be called ili9341.
> +
>  config TINYDRM_MI0283QT
>         tristate "DRM support for MI0283QT"
>         depends on DRM_TINYDRM && SPI
> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
> index 49a111929724..14d99080665a 100644
> --- a/drivers/gpu/drm/tinydrm/Makefile
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)          += mipi-dbi.o
>
>  # Displays
>  obj-$(CONFIG_TINYDRM_ILI9225)          += ili9225.o
> +obj-$(CONFIG_TINYDRM_ILI9341)          += ili9341.o
>  obj-$(CONFIG_TINYDRM_MI0283QT)         += mi0283qt.o
>  obj-$(CONFIG_TINYDRM_REPAPER)          += repaper.o
>  obj-$(CONFIG_TINYDRM_ST7586)           += st7586.o
> diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
> new file mode 100644
> index 000000000000..2ce4244a68c3
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/ili9341.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DRM driver for Ilitek ILI9341 panels
> + *
> + * Copyright 2018 David Lechner <david@lechnology.com>
> + *
> + * Based on mi0283qt.c:
> + * Copyright 2016 Noralf Trønnes
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_fb_helper.h>

> +#include <drm/drm_modeset_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>

Can it be in order?

> +#include <drm/tinydrm/mipi-dbi.h>
> +#include <drm/tinydrm/tinydrm-helpers.h>
> +#include <video/mipi_display.h>
> +
> +#define ILI9341_FRMCTR1                0xb1
> +#define ILI9341_DISCTRL                0xb6
> +#define ILI9341_ETMOD          0xb7
> +
> +#define ILI9341_PWCTRL1                0xc0
> +#define ILI9341_PWCTRL2                0xc1
> +#define ILI9341_VMCTRL1                0xc5
> +#define ILI9341_VMCTRL2                0xc7
> +#define ILI9341_PWCTRLA                0xcb
> +#define ILI9341_PWCTRLB                0xcf
> +
> +#define ILI9341_PGAMCTRL       0xe0
> +#define ILI9341_NGAMCTRL       0xe1
> +#define ILI9341_DTCTRLA                0xe8
> +#define ILI9341_DTCTRLB                0xea
> +#define ILI9341_PWRSEQ         0xed
> +
> +#define ILI9341_EN3GAM         0xf2
> +#define ILI9341_PUMPCTRL       0xf7
> +
> +#define ILI9341_MADCTL_BGR     BIT(3)
> +#define ILI9341_MADCTL_MV      BIT(5)
> +#define ILI9341_MADCTL_MX      BIT(6)
> +#define ILI9341_MADCTL_MY      BIT(7)
> +
> +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;
> +
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
> +
> +       mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
> +       mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
> +       mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
> +       mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
> +       mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
> +       mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
> +
> +       /* Power Control */
> +       mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
> +       mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
> +       /* VCOM */
> +       mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
> +       mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
> +
> +       /* Memory Access Control */
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
> +
> +       /* Frame Rate */
> +       mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
> +
> +       /* Gamma */
> +       mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> +       mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
> +                        0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
> +                        0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> +       mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
> +                        0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
> +                        0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
> +
> +       /* DDRAM */
> +       mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
> +
> +       /* Display */
> +       mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
> +       mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       msleep(100);
> +
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
> +       msleep(100);
> +
> +out_enable:
> +       switch (mipi->rotation) {
> +       default:
> +               addr_mode = ILI9341_MADCTL_MX;
> +               break;
> +       case 90:
> +               addr_mode = ILI9341_MADCTL_MV;
> +               break;
> +       case 180:
> +               addr_mode = ILI9341_MADCTL_MY;
> +               break;
> +       case 270:
> +               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
> +                           ILI9341_MADCTL_MX;
> +               break;
> +       }
> +       addr_mode |= ILI9341_MADCTL_BGR;
> +       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 ili9341_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 yx240qv29_mode = {
> +       TINYDRM_MODE(240, 320, 37, 49),
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
> +
> +static struct drm_driver ili9341_driver = {

> +       .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> +                                 DRIVER_ATOMIC,

I would rather keep one per line, or all at one line disregard 80
character limit.

> +       .fops                   = &ili9341_fops,
> +       TINYDRM_GEM_DRIVER_OPS,
> +       .lastclose              = drm_fb_helper_lastclose,
> +       .debugfs_init           = mipi_dbi_debugfs_init,
> +       .name                   = "ili9341",
> +       .desc                   = "Ilitek ILI9341",
> +       .date                   = "20180514",
> +       .major                  = 1,
> +       .minor                  = 0,
> +};
> +
> +static const struct of_device_id ili9341_of_match[] = {
> +       { .compatible = "noname,yx240qv29" },
> +       {},

Terminators better w/o comma.

> +};
> +MODULE_DEVICE_TABLE(of, ili9341_of_match);
> +
> +static const struct spi_device_id ili9341_id[] = {
> +       { "yx240qv29", 0 },
> +       { },

Ditto.

> +};
> +MODULE_DEVICE_TABLE(spi, ili9341_id);
> +
> +static int ili9341_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;
> +
> +       mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(mipi->reset)) {
> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
> +               return PTR_ERR(mipi->reset);
> +       }
> +
> +       dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
> +       if (IS_ERR(dc)) {
> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
> +               return PTR_ERR(dc);
> +       }
> +
> +       mipi->regulator = devm_regulator_get(dev, "power");
> +       if (IS_ERR(mipi->regulator))
> +               return PTR_ERR(mipi->regulator);
> +
> +       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, &ili9341_pipe_funcs,
> +                           &ili9341_driver, &yx240qv29_mode, rotation);
> +       if (ret)
> +               return ret;
> +
> +       spi_set_drvdata(spi, mipi);
> +
> +       return devm_tinydrm_register(&mipi->tinydrm);
> +}
> +
> +static void ili9341_shutdown(struct spi_device *spi)
> +{
> +       struct mipi_dbi *mipi = spi_get_drvdata(spi);
> +
> +       tinydrm_shutdown(&mipi->tinydrm);
> +}
> +
> +static struct spi_driver ili9341_spi_driver = {
> +       .driver = {
> +               .name = "ili9341",
> +               .of_match_table = ili9341_of_match,
> +       },
> +       .id_table = ili9341_id,
> +       .probe = ili9341_probe,
> +       .shutdown = ili9341_shutdown,
> +};
> +module_spi_driver(ili9341_spi_driver);
> +
> +MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.17.0
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
@ 2018-05-15 21:43     ` Andy Shevchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2018-05-15 21:43 UTC (permalink / raw)
  To: David Lechner
  Cc: Mark Rutland, devicetree, limor, Linux Kernel Mailing List,
	Rob Herring, dri-devel, Nitin Patil

On Tue, May 15, 2018 at 4:43 AM, David Lechner <david@lechnology.com> wrote:
> This adds a new driver for display panels that use the Ilitek ILI9341
> controller. It currently supports a single display panel, namely
> the YX240QV29-T (e.g. Adafruit 2.4" TFT).
>
> The init sequence is from the Adafruit Python library for the ILI9341
> controller. https://github.com/adafruit/Adafruit_Python_ILI9341

Some minor style nitpicks, otherwise LGTM

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  MAINTAINERS                       |   6 +
>  drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
>  drivers/gpu/drm/tinydrm/Makefile  |   1 +
>  drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
>  4 files changed, 256 insertions(+)
>  create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc219de9cbee..ffa099abbd79 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4480,6 +4480,12 @@ S:       Maintained
>  F:     drivers/gpu/drm/tinydrm/ili9225.c
>  F:     Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>
> +DRM DRIVER FOR ILITEK ILI9341 PANELS
> +M:     David Lechner <david@lechnology.com>
> +S:     Maintained
> +F:     drivers/gpu/drm/tinydrm/ili9341.c
> +F:     Documentation/devicetree/bindings/display/ilitek,ili9341.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 4592a5e3f20b..7a8008b0783f 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -20,6 +20,16 @@ config TINYDRM_ILI9225
>
>           If M is selected the module will be called ili9225.
>
> +config TINYDRM_ILI9341
> +       tristate "DRM support for ILI9341 display panels"

> +       depends on DRM_TINYDRM && SPI

Can't we do something like

if SPI

...

endif

?

> +       select TINYDRM_MIPI_DBI
> +       help
> +         DRM driver for the following Ilitek ILI9341 panels:
> +         * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
> +
> +         If M is selected the module will be called ili9341.
> +
>  config TINYDRM_MI0283QT
>         tristate "DRM support for MI0283QT"
>         depends on DRM_TINYDRM && SPI
> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
> index 49a111929724..14d99080665a 100644
> --- a/drivers/gpu/drm/tinydrm/Makefile
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)          += mipi-dbi.o
>
>  # Displays
>  obj-$(CONFIG_TINYDRM_ILI9225)          += ili9225.o
> +obj-$(CONFIG_TINYDRM_ILI9341)          += ili9341.o
>  obj-$(CONFIG_TINYDRM_MI0283QT)         += mi0283qt.o
>  obj-$(CONFIG_TINYDRM_REPAPER)          += repaper.o
>  obj-$(CONFIG_TINYDRM_ST7586)           += st7586.o
> diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
> new file mode 100644
> index 000000000000..2ce4244a68c3
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/ili9341.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DRM driver for Ilitek ILI9341 panels
> + *
> + * Copyright 2018 David Lechner <david@lechnology.com>
> + *
> + * Based on mi0283qt.c:
> + * Copyright 2016 Noralf Trønnes
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_fb_helper.h>

> +#include <drm/drm_modeset_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>

Can it be in order?

> +#include <drm/tinydrm/mipi-dbi.h>
> +#include <drm/tinydrm/tinydrm-helpers.h>
> +#include <video/mipi_display.h>
> +
> +#define ILI9341_FRMCTR1                0xb1
> +#define ILI9341_DISCTRL                0xb6
> +#define ILI9341_ETMOD          0xb7
> +
> +#define ILI9341_PWCTRL1                0xc0
> +#define ILI9341_PWCTRL2                0xc1
> +#define ILI9341_VMCTRL1                0xc5
> +#define ILI9341_VMCTRL2                0xc7
> +#define ILI9341_PWCTRLA                0xcb
> +#define ILI9341_PWCTRLB                0xcf
> +
> +#define ILI9341_PGAMCTRL       0xe0
> +#define ILI9341_NGAMCTRL       0xe1
> +#define ILI9341_DTCTRLA                0xe8
> +#define ILI9341_DTCTRLB                0xea
> +#define ILI9341_PWRSEQ         0xed
> +
> +#define ILI9341_EN3GAM         0xf2
> +#define ILI9341_PUMPCTRL       0xf7
> +
> +#define ILI9341_MADCTL_BGR     BIT(3)
> +#define ILI9341_MADCTL_MV      BIT(5)
> +#define ILI9341_MADCTL_MX      BIT(6)
> +#define ILI9341_MADCTL_MY      BIT(7)
> +
> +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;
> +
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
> +
> +       mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
> +       mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
> +       mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
> +       mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
> +       mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
> +       mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
> +
> +       /* Power Control */
> +       mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
> +       mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
> +       /* VCOM */
> +       mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
> +       mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
> +
> +       /* Memory Access Control */
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
> +
> +       /* Frame Rate */
> +       mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
> +
> +       /* Gamma */
> +       mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> +       mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
> +                        0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
> +                        0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> +       mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
> +                        0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
> +                        0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
> +
> +       /* DDRAM */
> +       mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
> +
> +       /* Display */
> +       mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
> +       mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       msleep(100);
> +
> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
> +       msleep(100);
> +
> +out_enable:
> +       switch (mipi->rotation) {
> +       default:
> +               addr_mode = ILI9341_MADCTL_MX;
> +               break;
> +       case 90:
> +               addr_mode = ILI9341_MADCTL_MV;
> +               break;
> +       case 180:
> +               addr_mode = ILI9341_MADCTL_MY;
> +               break;
> +       case 270:
> +               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
> +                           ILI9341_MADCTL_MX;
> +               break;
> +       }
> +       addr_mode |= ILI9341_MADCTL_BGR;
> +       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 ili9341_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 yx240qv29_mode = {
> +       TINYDRM_MODE(240, 320, 37, 49),
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
> +
> +static struct drm_driver ili9341_driver = {

> +       .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> +                                 DRIVER_ATOMIC,

I would rather keep one per line, or all at one line disregard 80
character limit.

> +       .fops                   = &ili9341_fops,
> +       TINYDRM_GEM_DRIVER_OPS,
> +       .lastclose              = drm_fb_helper_lastclose,
> +       .debugfs_init           = mipi_dbi_debugfs_init,
> +       .name                   = "ili9341",
> +       .desc                   = "Ilitek ILI9341",
> +       .date                   = "20180514",
> +       .major                  = 1,
> +       .minor                  = 0,
> +};
> +
> +static const struct of_device_id ili9341_of_match[] = {
> +       { .compatible = "noname,yx240qv29" },
> +       {},

Terminators better w/o comma.

> +};
> +MODULE_DEVICE_TABLE(of, ili9341_of_match);
> +
> +static const struct spi_device_id ili9341_id[] = {
> +       { "yx240qv29", 0 },
> +       { },

Ditto.

> +};
> +MODULE_DEVICE_TABLE(spi, ili9341_id);
> +
> +static int ili9341_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;
> +
> +       mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(mipi->reset)) {
> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
> +               return PTR_ERR(mipi->reset);
> +       }
> +
> +       dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
> +       if (IS_ERR(dc)) {
> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
> +               return PTR_ERR(dc);
> +       }
> +
> +       mipi->regulator = devm_regulator_get(dev, "power");
> +       if (IS_ERR(mipi->regulator))
> +               return PTR_ERR(mipi->regulator);
> +
> +       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, &ili9341_pipe_funcs,
> +                           &ili9341_driver, &yx240qv29_mode, rotation);
> +       if (ret)
> +               return ret;
> +
> +       spi_set_drvdata(spi, mipi);
> +
> +       return devm_tinydrm_register(&mipi->tinydrm);
> +}
> +
> +static void ili9341_shutdown(struct spi_device *spi)
> +{
> +       struct mipi_dbi *mipi = spi_get_drvdata(spi);
> +
> +       tinydrm_shutdown(&mipi->tinydrm);
> +}
> +
> +static struct spi_driver ili9341_spi_driver = {
> +       .driver = {
> +               .name = "ili9341",
> +               .of_match_table = ili9341_of_match,
> +       },
> +       .id_table = ili9341_id,
> +       .probe = ili9341_probe,
> +       .shutdown = ili9341_shutdown,
> +};
> +module_spi_driver(ili9341_spi_driver);
> +
> +MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.17.0
>



-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
  2018-05-15  1:43   ` [PATCH 1/3] MAINTAINERS: fix path to ilitek, ili9225 " David Lechner
@ 2018-05-19 13:24     ` Noralf Trønnes
  -1 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2018-05-19 13:24 UTC (permalink / raw)
  To: David Lechner, dri-devel, devicetree
  Cc: Rob Herring, Mark Rutland, limor, Nitin Patil, linux-kernel


Den 15.05.2018 03.43, skrev David Lechner:
> This fixes the path to the ilitek,ili9225 device tree binding file.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

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

>   MAINTAINERS | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 334a00350922..bc219de9cbee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4478,7 +4478,7 @@ DRM DRIVER FOR ILITEK ILI9225 PANELS
>   M:	David Lechner <david@lechnology.com>
>   S:	Maintained
>   F:	drivers/gpu/drm/tinydrm/ili9225.c
> -F:	Documentation/devicetree/bindings/display/ili9225.txt
> +F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>   
>   DRM DRIVER FOR INTEL I810 VIDEO CARDS
>   S:	Orphan / Obsolete

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
@ 2018-05-19 13:24     ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2018-05-19 13:24 UTC (permalink / raw)
  To: David Lechner, dri-devel, devicetree
  Cc: Mark Rutland, Nitin Patil, Rob Herring, limor, linux-kernel


Den 15.05.2018 03.43, skrev David Lechner:
> This fixes the path to the ilitek,ili9225 device tree binding file.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

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

>   MAINTAINERS | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 334a00350922..bc219de9cbee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4478,7 +4478,7 @@ DRM DRIVER FOR ILITEK ILI9225 PANELS
>   M:	David Lechner <david@lechnology.com>
>   S:	Maintained
>   F:	drivers/gpu/drm/tinydrm/ili9225.c
> -F:	Documentation/devicetree/bindings/display/ili9225.txt
> +F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>   
>   DRM DRIVER FOR INTEL I810 VIDEO CARDS
>   S:	Orphan / Obsolete

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels
  2018-05-15  1:43   ` David Lechner
@ 2018-05-19 13:37     ` Noralf Trønnes
  -1 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2018-05-19 13:37 UTC (permalink / raw)
  To: David Lechner, dri-devel, devicetree
  Cc: Rob Herring, Mark Rutland, limor, Nitin Patil, linux-kernel


Den 15.05.2018 03.43, skrev David Lechner:
> This adds a new binding for Ilitek ILI9341 display panels. It includes
> a compatible string for one display (more can be added in the future).
>
> The vendor prefix "noname" is used because the vendor is not known.
> The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
> board[2] and in Mindsensors' PiStorms[3].
>
> [1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
> [2]: https://www.adafruit.com/product/2478
> [3]: http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>   .../bindings/display/ilitek,ili9341.txt       | 27 +++++++++++++++++++
>   1 file changed, 27 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>
> diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> new file mode 100644
> index 000000000000..0fc90b2dd732
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> @@ -0,0 +1,27 @@
> +Ilitek ILI9341 display panels
> +
> +This binding is for display panels using an Ilitek ILI9341 controller in SPI
> +mode.
> +
> +Required properties:
> +- compatible:	"noname,yx240qv29", "ilitek,ili9341"

Calling a vendor 'noname' looks a bit odd and AFAICT it isn't used by
any other binding. A google search always mentions Adafruit in
connection with this panel, so I suggest we use Adafruit as vendor.

I don't think we should use "ilitek,ili9341" as an option/fallback,
because panels varies in resolution (rarely) and initialization. A
generic ili9341 driver would probably not work with a random new panel.

> +- dc-gpios:	D/C pin
> +- reset-gpios:	Reset 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

You forgot to mention the regulator that you support in the driver.

Noralf.

> +
> +Example:
> +	display@0{
> +		compatible = "noname,yx240qv29", "ilitek,ili9341";
> +		reg = <0>;
> +		spi-max-frequency = <32000000>;
> +		dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> +		rotation = <270>;
> +		backlight = <&backlight>;
> +	};

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels
@ 2018-05-19 13:37     ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2018-05-19 13:37 UTC (permalink / raw)
  To: David Lechner, dri-devel, devicetree
  Cc: Mark Rutland, Nitin Patil, Rob Herring, limor, linux-kernel


Den 15.05.2018 03.43, skrev David Lechner:
> This adds a new binding for Ilitek ILI9341 display panels. It includes
> a compatible string for one display (more can be added in the future).
>
> The vendor prefix "noname" is used because the vendor is not known.
> The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
> board[2] and in Mindsensors' PiStorms[3].
>
> [1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
> [2]: https://www.adafruit.com/product/2478
> [3]: http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>   .../bindings/display/ilitek,ili9341.txt       | 27 +++++++++++++++++++
>   1 file changed, 27 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>
> diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> new file mode 100644
> index 000000000000..0fc90b2dd732
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> @@ -0,0 +1,27 @@
> +Ilitek ILI9341 display panels
> +
> +This binding is for display panels using an Ilitek ILI9341 controller in SPI
> +mode.
> +
> +Required properties:
> +- compatible:	"noname,yx240qv29", "ilitek,ili9341"

Calling a vendor 'noname' looks a bit odd and AFAICT it isn't used by
any other binding. A google search always mentions Adafruit in
connection with this panel, so I suggest we use Adafruit as vendor.

I don't think we should use "ilitek,ili9341" as an option/fallback,
because panels varies in resolution (rarely) and initialization. A
generic ili9341 driver would probably not work with a random new panel.

> +- dc-gpios:	D/C pin
> +- reset-gpios:	Reset 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

You forgot to mention the regulator that you support in the driver.

Noralf.

> +
> +Example:
> +	display@0{
> +		compatible = "noname,yx240qv29", "ilitek,ili9341";
> +		reg = <0>;
> +		spi-max-frequency = <32000000>;
> +		dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> +		rotation = <270>;
> +		backlight = <&backlight>;
> +	};

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
  2018-05-15 21:43     ` Andy Shevchenko
@ 2018-05-19 14:00       ` Noralf Trønnes
  -1 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2018-05-19 14:00 UTC (permalink / raw)
  To: Andy Shevchenko, David Lechner
  Cc: dri-devel, devicetree, Rob Herring, Mark Rutland, limor,
	Nitin Patil, Linux Kernel Mailing List


Den 15.05.2018 23.43, skrev Andy Shevchenko:
> On Tue, May 15, 2018 at 4:43 AM, David Lechner <david@lechnology.com> wrote:
>> This adds a new driver for display panels that use the Ilitek ILI9341
>> controller. It currently supports a single display panel, namely
>> the YX240QV29-T (e.g. Adafruit 2.4" TFT).
>>
>> The init sequence is from the Adafruit Python library for the ILI9341
>> controller. https://github.com/adafruit/Adafruit_Python_ILI9341
> Some minor style nitpicks, otherwise LGTM
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>   MAINTAINERS                       |   6 +
>>   drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
>>   drivers/gpu/drm/tinydrm/Makefile  |   1 +
>>   drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
>>   4 files changed, 256 insertions(+)
>>   create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index bc219de9cbee..ffa099abbd79 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4480,6 +4480,12 @@ S:       Maintained
>>   F:     drivers/gpu/drm/tinydrm/ili9225.c
>>   F:     Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>>
>> +DRM DRIVER FOR ILITEK ILI9341 PANELS
>> +M:     David Lechner <david@lechnology.com>
>> +S:     Maintained
>> +F:     drivers/gpu/drm/tinydrm/ili9341.c
>> +F:     Documentation/devicetree/bindings/display/ilitek,ili9341.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 4592a5e3f20b..7a8008b0783f 100644
>> --- a/drivers/gpu/drm/tinydrm/Kconfig
>> +++ b/drivers/gpu/drm/tinydrm/Kconfig
>> @@ -20,6 +20,16 @@ config TINYDRM_ILI9225
>>
>>            If M is selected the module will be called ili9225.
>>
>> +config TINYDRM_ILI9341
>> +       tristate "DRM support for ILI9341 display panels"
>> +       depends on DRM_TINYDRM && SPI
> Can't we do something like
>
> if SPI
>
> ...
>
> endif
>
> ?
>
>> +       select TINYDRM_MIPI_DBI
>> +       help
>> +         DRM driver for the following Ilitek ILI9341 panels:
>> +         * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
>> +
>> +         If M is selected the module will be called ili9341.
>> +
>>   config TINYDRM_MI0283QT
>>          tristate "DRM support for MI0283QT"
>>          depends on DRM_TINYDRM && SPI
>> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
>> index 49a111929724..14d99080665a 100644
>> --- a/drivers/gpu/drm/tinydrm/Makefile
>> +++ b/drivers/gpu/drm/tinydrm/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)          += mipi-dbi.o
>>
>>   # Displays
>>   obj-$(CONFIG_TINYDRM_ILI9225)          += ili9225.o
>> +obj-$(CONFIG_TINYDRM_ILI9341)          += ili9341.o
>>   obj-$(CONFIG_TINYDRM_MI0283QT)         += mi0283qt.o
>>   obj-$(CONFIG_TINYDRM_REPAPER)          += repaper.o
>>   obj-$(CONFIG_TINYDRM_ST7586)           += st7586.o
>> diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
>> new file mode 100644
>> index 000000000000..2ce4244a68c3
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tinydrm/ili9341.c
>> @@ -0,0 +1,239 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * DRM driver for Ilitek ILI9341 panels
>> + *
>> + * Copyright 2018 David Lechner <david@lechnology.com>
>> + *
>> + * Based on mi0283qt.c:
>> + * Copyright 2016 Noralf Trønnes
>> + */
>> +
>> +#include <linux/backlight.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/property.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include <drm/drm_fb_helper.h>
>> +#include <drm/drm_modeset_helper.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
> Can it be in order?
>
>> +#include <drm/tinydrm/mipi-dbi.h>
>> +#include <drm/tinydrm/tinydrm-helpers.h>
>> +#include <video/mipi_display.h>
>> +
>> +#define ILI9341_FRMCTR1                0xb1
>> +#define ILI9341_DISCTRL                0xb6
>> +#define ILI9341_ETMOD          0xb7
>> +
>> +#define ILI9341_PWCTRL1                0xc0
>> +#define ILI9341_PWCTRL2                0xc1
>> +#define ILI9341_VMCTRL1                0xc5
>> +#define ILI9341_VMCTRL2                0xc7
>> +#define ILI9341_PWCTRLA                0xcb
>> +#define ILI9341_PWCTRLB                0xcf
>> +
>> +#define ILI9341_PGAMCTRL       0xe0
>> +#define ILI9341_NGAMCTRL       0xe1
>> +#define ILI9341_DTCTRLA                0xe8
>> +#define ILI9341_DTCTRLB                0xea
>> +#define ILI9341_PWRSEQ         0xed
>> +
>> +#define ILI9341_EN3GAM         0xf2
>> +#define ILI9341_PUMPCTRL       0xf7
>> +
>> +#define ILI9341_MADCTL_BGR     BIT(3)
>> +#define ILI9341_MADCTL_MV      BIT(5)
>> +#define ILI9341_MADCTL_MX      BIT(6)
>> +#define ILI9341_MADCTL_MY      BIT(7)
>> +
>> +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;
>> +
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
>> +
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
>> +       mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
>> +       mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
>> +       mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
>> +       mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
>> +
>> +       /* Power Control */
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
>> +       /* VCOM */
>> +       mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
>> +       mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
>> +
>> +       /* Memory Access Control */
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
>> +
>> +       /* Frame Rate */
>> +       mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
>> +
>> +       /* Gamma */
>> +       mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
>> +       mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
>> +                        0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
>> +                        0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
>> +       mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
>> +                        0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
>> +                        0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
>> +
>> +       /* DDRAM */
>> +       mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
>> +
>> +       /* Display */
>> +       mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
>> +       mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
>> +       msleep(100);
>> +
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
>> +       msleep(100);
>> +
>> +out_enable:
>> +       switch (mipi->rotation) {
>> +       default:
>> +               addr_mode = ILI9341_MADCTL_MX;
>> +               break;
>> +       case 90:
>> +               addr_mode = ILI9341_MADCTL_MV;
>> +               break;
>> +       case 180:
>> +               addr_mode = ILI9341_MADCTL_MY;
>> +               break;
>> +       case 270:
>> +               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
>> +                           ILI9341_MADCTL_MX;
>> +               break;
>> +       }
>> +       addr_mode |= ILI9341_MADCTL_BGR;
>> +       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 ili9341_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 yx240qv29_mode = {
>> +       TINYDRM_MODE(240, 320, 37, 49),
>> +};
>> +
>> +DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
>> +
>> +static struct drm_driver ili9341_driver = {
>> +       .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
>> +                                 DRIVER_ATOMIC,
> I would rather keep one per line, or all at one line disregard 80
> character limit.

I prefer on one line. I've started to use 100 character limit when it
makes sense, because all these linebreaks makes the code harder to read
(checkpatch.pl --max-line-length=100).

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

Noralf.

>> +       .fops                   = &ili9341_fops,
>> +       TINYDRM_GEM_DRIVER_OPS,
>> +       .lastclose              = drm_fb_helper_lastclose,
>> +       .debugfs_init           = mipi_dbi_debugfs_init,
>> +       .name                   = "ili9341",
>> +       .desc                   = "Ilitek ILI9341",
>> +       .date                   = "20180514",
>> +       .major                  = 1,
>> +       .minor                  = 0,
>> +};
>> +
>> +static const struct of_device_id ili9341_of_match[] = {
>> +       { .compatible = "noname,yx240qv29" },
>> +       {},
> Terminators better w/o comma.
>
>> +};
>> +MODULE_DEVICE_TABLE(of, ili9341_of_match);
>> +
>> +static const struct spi_device_id ili9341_id[] = {
>> +       { "yx240qv29", 0 },
>> +       { },
> Ditto.
>
>> +};
>> +MODULE_DEVICE_TABLE(spi, ili9341_id);
>> +
>> +static int ili9341_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;
>> +
>> +       mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> +       if (IS_ERR(mipi->reset)) {
>> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
>> +               return PTR_ERR(mipi->reset);
>> +       }
>> +
>> +       dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
>> +       if (IS_ERR(dc)) {
>> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
>> +               return PTR_ERR(dc);
>> +       }
>> +
>> +       mipi->regulator = devm_regulator_get(dev, "power");
>> +       if (IS_ERR(mipi->regulator))
>> +               return PTR_ERR(mipi->regulator);
>> +
>> +       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, &ili9341_pipe_funcs,
>> +                           &ili9341_driver, &yx240qv29_mode, rotation);
>> +       if (ret)
>> +               return ret;
>> +
>> +       spi_set_drvdata(spi, mipi);
>> +
>> +       return devm_tinydrm_register(&mipi->tinydrm);
>> +}
>> +
>> +static void ili9341_shutdown(struct spi_device *spi)
>> +{
>> +       struct mipi_dbi *mipi = spi_get_drvdata(spi);
>> +
>> +       tinydrm_shutdown(&mipi->tinydrm);
>> +}
>> +
>> +static struct spi_driver ili9341_spi_driver = {
>> +       .driver = {
>> +               .name = "ili9341",
>> +               .of_match_table = ili9341_of_match,
>> +       },
>> +       .id_table = ili9341_id,
>> +       .probe = ili9341_probe,
>> +       .shutdown = ili9341_shutdown,
>> +};
>> +module_spi_driver(ili9341_spi_driver);
>> +
>> +MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
>> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.17.0
>>
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
@ 2018-05-19 14:00       ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2018-05-19 14:00 UTC (permalink / raw)
  To: Andy Shevchenko, David Lechner
  Cc: Mark Rutland, devicetree, limor, Linux Kernel Mailing List,
	dri-devel, Rob Herring, Nitin Patil


Den 15.05.2018 23.43, skrev Andy Shevchenko:
> On Tue, May 15, 2018 at 4:43 AM, David Lechner <david@lechnology.com> wrote:
>> This adds a new driver for display panels that use the Ilitek ILI9341
>> controller. It currently supports a single display panel, namely
>> the YX240QV29-T (e.g. Adafruit 2.4" TFT).
>>
>> The init sequence is from the Adafruit Python library for the ILI9341
>> controller. https://github.com/adafruit/Adafruit_Python_ILI9341
> Some minor style nitpicks, otherwise LGTM
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>   MAINTAINERS                       |   6 +
>>   drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
>>   drivers/gpu/drm/tinydrm/Makefile  |   1 +
>>   drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
>>   4 files changed, 256 insertions(+)
>>   create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index bc219de9cbee..ffa099abbd79 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4480,6 +4480,12 @@ S:       Maintained
>>   F:     drivers/gpu/drm/tinydrm/ili9225.c
>>   F:     Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>>
>> +DRM DRIVER FOR ILITEK ILI9341 PANELS
>> +M:     David Lechner <david@lechnology.com>
>> +S:     Maintained
>> +F:     drivers/gpu/drm/tinydrm/ili9341.c
>> +F:     Documentation/devicetree/bindings/display/ilitek,ili9341.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 4592a5e3f20b..7a8008b0783f 100644
>> --- a/drivers/gpu/drm/tinydrm/Kconfig
>> +++ b/drivers/gpu/drm/tinydrm/Kconfig
>> @@ -20,6 +20,16 @@ config TINYDRM_ILI9225
>>
>>            If M is selected the module will be called ili9225.
>>
>> +config TINYDRM_ILI9341
>> +       tristate "DRM support for ILI9341 display panels"
>> +       depends on DRM_TINYDRM && SPI
> Can't we do something like
>
> if SPI
>
> ...
>
> endif
>
> ?
>
>> +       select TINYDRM_MIPI_DBI
>> +       help
>> +         DRM driver for the following Ilitek ILI9341 panels:
>> +         * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
>> +
>> +         If M is selected the module will be called ili9341.
>> +
>>   config TINYDRM_MI0283QT
>>          tristate "DRM support for MI0283QT"
>>          depends on DRM_TINYDRM && SPI
>> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
>> index 49a111929724..14d99080665a 100644
>> --- a/drivers/gpu/drm/tinydrm/Makefile
>> +++ b/drivers/gpu/drm/tinydrm/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)          += mipi-dbi.o
>>
>>   # Displays
>>   obj-$(CONFIG_TINYDRM_ILI9225)          += ili9225.o
>> +obj-$(CONFIG_TINYDRM_ILI9341)          += ili9341.o
>>   obj-$(CONFIG_TINYDRM_MI0283QT)         += mi0283qt.o
>>   obj-$(CONFIG_TINYDRM_REPAPER)          += repaper.o
>>   obj-$(CONFIG_TINYDRM_ST7586)           += st7586.o
>> diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
>> new file mode 100644
>> index 000000000000..2ce4244a68c3
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tinydrm/ili9341.c
>> @@ -0,0 +1,239 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * DRM driver for Ilitek ILI9341 panels
>> + *
>> + * Copyright 2018 David Lechner <david@lechnology.com>
>> + *
>> + * Based on mi0283qt.c:
>> + * Copyright 2016 Noralf Trønnes
>> + */
>> +
>> +#include <linux/backlight.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/property.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include <drm/drm_fb_helper.h>
>> +#include <drm/drm_modeset_helper.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
> Can it be in order?
>
>> +#include <drm/tinydrm/mipi-dbi.h>
>> +#include <drm/tinydrm/tinydrm-helpers.h>
>> +#include <video/mipi_display.h>
>> +
>> +#define ILI9341_FRMCTR1                0xb1
>> +#define ILI9341_DISCTRL                0xb6
>> +#define ILI9341_ETMOD          0xb7
>> +
>> +#define ILI9341_PWCTRL1                0xc0
>> +#define ILI9341_PWCTRL2                0xc1
>> +#define ILI9341_VMCTRL1                0xc5
>> +#define ILI9341_VMCTRL2                0xc7
>> +#define ILI9341_PWCTRLA                0xcb
>> +#define ILI9341_PWCTRLB                0xcf
>> +
>> +#define ILI9341_PGAMCTRL       0xe0
>> +#define ILI9341_NGAMCTRL       0xe1
>> +#define ILI9341_DTCTRLA                0xe8
>> +#define ILI9341_DTCTRLB                0xea
>> +#define ILI9341_PWRSEQ         0xed
>> +
>> +#define ILI9341_EN3GAM         0xf2
>> +#define ILI9341_PUMPCTRL       0xf7
>> +
>> +#define ILI9341_MADCTL_BGR     BIT(3)
>> +#define ILI9341_MADCTL_MV      BIT(5)
>> +#define ILI9341_MADCTL_MX      BIT(6)
>> +#define ILI9341_MADCTL_MY      BIT(7)
>> +
>> +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;
>> +
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
>> +
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
>> +       mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
>> +       mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
>> +       mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
>> +       mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
>> +
>> +       /* Power Control */
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
>> +       mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
>> +       /* VCOM */
>> +       mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
>> +       mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
>> +
>> +       /* Memory Access Control */
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
>> +
>> +       /* Frame Rate */
>> +       mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
>> +
>> +       /* Gamma */
>> +       mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
>> +       mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
>> +                        0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
>> +                        0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
>> +       mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
>> +                        0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
>> +                        0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
>> +
>> +       /* DDRAM */
>> +       mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
>> +
>> +       /* Display */
>> +       mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
>> +       mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
>> +       msleep(100);
>> +
>> +       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
>> +       msleep(100);
>> +
>> +out_enable:
>> +       switch (mipi->rotation) {
>> +       default:
>> +               addr_mode = ILI9341_MADCTL_MX;
>> +               break;
>> +       case 90:
>> +               addr_mode = ILI9341_MADCTL_MV;
>> +               break;
>> +       case 180:
>> +               addr_mode = ILI9341_MADCTL_MY;
>> +               break;
>> +       case 270:
>> +               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
>> +                           ILI9341_MADCTL_MX;
>> +               break;
>> +       }
>> +       addr_mode |= ILI9341_MADCTL_BGR;
>> +       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 ili9341_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 yx240qv29_mode = {
>> +       TINYDRM_MODE(240, 320, 37, 49),
>> +};
>> +
>> +DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
>> +
>> +static struct drm_driver ili9341_driver = {
>> +       .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
>> +                                 DRIVER_ATOMIC,
> I would rather keep one per line, or all at one line disregard 80
> character limit.

I prefer on one line. I've started to use 100 character limit when it
makes sense, because all these linebreaks makes the code harder to read
(checkpatch.pl --max-line-length=100).

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

Noralf.

>> +       .fops                   = &ili9341_fops,
>> +       TINYDRM_GEM_DRIVER_OPS,
>> +       .lastclose              = drm_fb_helper_lastclose,
>> +       .debugfs_init           = mipi_dbi_debugfs_init,
>> +       .name                   = "ili9341",
>> +       .desc                   = "Ilitek ILI9341",
>> +       .date                   = "20180514",
>> +       .major                  = 1,
>> +       .minor                  = 0,
>> +};
>> +
>> +static const struct of_device_id ili9341_of_match[] = {
>> +       { .compatible = "noname,yx240qv29" },
>> +       {},
> Terminators better w/o comma.
>
>> +};
>> +MODULE_DEVICE_TABLE(of, ili9341_of_match);
>> +
>> +static const struct spi_device_id ili9341_id[] = {
>> +       { "yx240qv29", 0 },
>> +       { },
> Ditto.
>
>> +};
>> +MODULE_DEVICE_TABLE(spi, ili9341_id);
>> +
>> +static int ili9341_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;
>> +
>> +       mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> +       if (IS_ERR(mipi->reset)) {
>> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
>> +               return PTR_ERR(mipi->reset);
>> +       }
>> +
>> +       dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
>> +       if (IS_ERR(dc)) {
>> +               DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
>> +               return PTR_ERR(dc);
>> +       }
>> +
>> +       mipi->regulator = devm_regulator_get(dev, "power");
>> +       if (IS_ERR(mipi->regulator))
>> +               return PTR_ERR(mipi->regulator);
>> +
>> +       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, &ili9341_pipe_funcs,
>> +                           &ili9341_driver, &yx240qv29_mode, rotation);
>> +       if (ret)
>> +               return ret;
>> +
>> +       spi_set_drvdata(spi, mipi);
>> +
>> +       return devm_tinydrm_register(&mipi->tinydrm);
>> +}
>> +
>> +static void ili9341_shutdown(struct spi_device *spi)
>> +{
>> +       struct mipi_dbi *mipi = spi_get_drvdata(spi);
>> +
>> +       tinydrm_shutdown(&mipi->tinydrm);
>> +}
>> +
>> +static struct spi_driver ili9341_spi_driver = {
>> +       .driver = {
>> +               .name = "ili9341",
>> +               .of_match_table = ili9341_of_match,
>> +       },
>> +       .id_table = ili9341_id,
>> +       .probe = ili9341_probe,
>> +       .shutdown = ili9341_shutdown,
>> +};
>> +module_spi_driver(ili9341_spi_driver);
>> +
>> +MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
>> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.17.0
>>
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels
  2018-05-19 13:37     ` Noralf Trønnes
@ 2018-05-19 15:50       ` David Lechner
  -1 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-19 15:50 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel, devicetree
  Cc: Rob Herring, Mark Rutland, limor, Nitin Patil, linux-kernel

On 05/19/2018 08:37 AM, Noralf Trønnes wrote:
> 
> Den 15.05.2018 03.43, skrev David Lechner:
>> This adds a new binding for Ilitek ILI9341 display panels. It includes
>> a compatible string for one display (more can be added in the future).
>>
>> The vendor prefix "noname" is used because the vendor is not known.
>> The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
>> board[2] and in Mindsensors' PiStorms[3].
>>
>> [1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
>> [2]: https://www.adafruit.com/product/2478
>> [3]: http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>   .../bindings/display/ilitek,ili9341.txt       | 27 +++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>> new file mode 100644
>> index 000000000000..0fc90b2dd732
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>> @@ -0,0 +1,27 @@
>> +Ilitek ILI9341 display panels
>> +
>> +This binding is for display panels using an Ilitek ILI9341 controller in SPI
>> +mode.
>> +
>> +Required properties:
>> +- compatible:    "noname,yx240qv29", "ilitek,ili9341"
> 
> Calling a vendor 'noname' looks a bit odd and AFAICT it isn't used by
> any other binding. A google search always mentions Adafruit in
> connection with this panel, so I suggest we use Adafruit as vendor.

I was hoping that someone might know the correct vendor, but barring that,
I agree that adafruit is probably the best choice.

> 
> I don't think we should use "ilitek,ili9341" as an option/fallback,
> because panels varies in resolution (rarely) and initialization. A
> generic ili9341 driver would probably not work with a random new panel.

I'm just following the precedent set in the other bindings for similar
displays that I have already done.

References:
* https://patchwork.freedesktop.org/patch/194648/
* https://patchwork.freedesktop.org/patch/195320/

I agree that it is probably not super-useful as a fallback. On the other
hand, the vendors and models for these displays are pretty obscure and
I think having the more well-known controller name _somewhere_ is useful.


> 
>> +- dc-gpios:    D/C pin
>> +- reset-gpios:    Reset 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
> 
> You forgot to mention the regulator that you support in the driver.
> 
> Noralf.
> 
>> +
>> +Example:
>> +    display@0{
>> +        compatible = "noname,yx240qv29", "ilitek,ili9341";
>> +        reg = <0>;
>> +        spi-max-frequency = <32000000>;
>> +        dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>> +        reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>> +        rotation = <270>;
>> +        backlight = <&backlight>;
>> +    };
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels
@ 2018-05-19 15:50       ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2018-05-19 15:50 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel, devicetree
  Cc: Mark Rutland, Nitin Patil, Rob Herring, limor, linux-kernel

On 05/19/2018 08:37 AM, Noralf Trønnes wrote:
> 
> Den 15.05.2018 03.43, skrev David Lechner:
>> This adds a new binding for Ilitek ILI9341 display panels. It includes
>> a compatible string for one display (more can be added in the future).
>>
>> The vendor prefix "noname" is used because the vendor is not known.
>> The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
>> board[2] and in Mindsensors' PiStorms[3].
>>
>> [1]: https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
>> [2]: https://www.adafruit.com/product/2478
>> [3]: http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>   .../bindings/display/ilitek,ili9341.txt       | 27 +++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>> new file mode 100644
>> index 000000000000..0fc90b2dd732
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
>> @@ -0,0 +1,27 @@
>> +Ilitek ILI9341 display panels
>> +
>> +This binding is for display panels using an Ilitek ILI9341 controller in SPI
>> +mode.
>> +
>> +Required properties:
>> +- compatible:    "noname,yx240qv29", "ilitek,ili9341"
> 
> Calling a vendor 'noname' looks a bit odd and AFAICT it isn't used by
> any other binding. A google search always mentions Adafruit in
> connection with this panel, so I suggest we use Adafruit as vendor.

I was hoping that someone might know the correct vendor, but barring that,
I agree that adafruit is probably the best choice.

> 
> I don't think we should use "ilitek,ili9341" as an option/fallback,
> because panels varies in resolution (rarely) and initialization. A
> generic ili9341 driver would probably not work with a random new panel.

I'm just following the precedent set in the other bindings for similar
displays that I have already done.

References:
* https://patchwork.freedesktop.org/patch/194648/
* https://patchwork.freedesktop.org/patch/195320/

I agree that it is probably not super-useful as a fallback. On the other
hand, the vendors and models for these displays are pretty obscure and
I think having the more well-known controller name _somewhere_ is useful.


> 
>> +- dc-gpios:    D/C pin
>> +- reset-gpios:    Reset 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
> 
> You forgot to mention the regulator that you support in the driver.
> 
> Noralf.
> 
>> +
>> +Example:
>> +    display@0{
>> +        compatible = "noname,yx240qv29", "ilitek,ili9341";
>> +        reg = <0>;
>> +        spi-max-frequency = <32000000>;
>> +        dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>> +        reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>> +        rotation = <270>;
>> +        backlight = <&backlight>;
>> +    };
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-05-19 15:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-15  1:43 [PATCH 0/3] drm/tinydrm: new dirver for ILI9341 displays David Lechner
2018-05-15  1:43 ` David Lechner
2018-05-15  1:43 ` [PATCH 1/3] MAINTAINERS: fix path to ilitek,ili9225 device tree bindings David Lechner
2018-05-15  1:43   ` [PATCH 1/3] MAINTAINERS: fix path to ilitek, ili9225 " David Lechner
2018-05-19 13:24   ` [PATCH 1/3] MAINTAINERS: fix path to ilitek,ili9225 " Noralf Trønnes
2018-05-19 13:24     ` Noralf Trønnes
2018-05-15  1:43 ` [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels David Lechner
2018-05-15  1:43   ` David Lechner
2018-05-19 13:37   ` Noralf Trønnes
2018-05-19 13:37     ` Noralf Trønnes
2018-05-19 15:50     ` David Lechner
2018-05-19 15:50       ` David Lechner
2018-05-15  1:43 ` [PATCH 3/3] drm/tinydrm: new driver for " David Lechner
2018-05-15  1:43   ` David Lechner
2018-05-15 21:43   ` Andy Shevchenko
2018-05-15 21:43     ` Andy Shevchenko
2018-05-19 14:00     ` Noralf Trønnes
2018-05-19 14:00       ` Noralf Trønnes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.