All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] DRM driver for ILI9225 display panels
@ 2017-11-08  3:52 ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2017-11-08  3:52 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	linux-kernel

This is a new driver for ILI9225 based display panels.

There are a couple of things that stand out:

1.  Despite my best efforts, I could not find a name for this display[1], so I
    have made up a generic name for it. If someone recognizes this and has a
    name for it, please speak up. The best documentation I could find was
    here[2].

2.  There is quite a bit of one-off code copied from mipi-dbi.c. Based on the
    feedback from a previous patch series, I'm guessing that we don't want to
    modify mipi-dbi.c to accommodate the ILI9225 controller because the ILI9225
    controller does not use standard MIPI commands. ILI9225 has it's own
    non-standard command set, but other than that, it pretty much matches the
    generic mipi-dbi driver in all regards.

    Code that could be easily shared:

    a.  ili9225_buf_copy() is exactly the same as mipi_dbi_buf_copy()
        - ili9225_buf_copy() is used in ili9225_fb_dirty()
        - mipi_dbi_buf_copy() is static in mipi-dbi.c

    b.  ili9225_spi_cmd_max_speed() is exactly the same as
        mipi_dbi_spi_cmd_max_speed()
        - ili9225_spi_cmd_max_speed() is used in ili9225_command() below, so
          would not need to be copied if mipi_dbi_typec3_command() could be
          shared
        - mipi_dbi_spi_cmd_max_speed() is static in mipi-dbi.c

    c.  ili9225_command() is nearly the same as mipi_dbi_typec3_command()
        - a few unused lines were removed so I didn't have to copy even more
          code, but these would not be an issue if code was shared
        - cmd == ILI9225_WRITE_DATA_TO_GRAM instead of
          cmd == MIPI_DCS_WRITE_MEMORY_START

    d.  ili9225_spi_init() is nearly the same as mipi_dbi_spi_init()
        - a few unused lines were removed so I didn't have to copy even more
          code, but these would not be an issue if code was shared
        - mipi->command = ili9225_command instead of
          mipi->command = mipi_dbi_typec3_command
        - this function would not need to be copied if mipi_dbi_typec3_command()
          was modified to work with the ILI9225 command set too

    e.  ili9225_init() is nearly the same as mipi_dbi_init()
        - only difference is ili9225_fb_funcs instead of mipi_dbi_fb_funcs

3.  I haven't tried it, but I believe that it is possible to implement
    DRM_FORMAT_RGB888 directly instead of simulating DRM_FORMAT_XRGB8888.
    I don't know if there would be any real benefit to doing this. I am not
    familiar enough with userspace programs/libraries to know if this mode is
    universally used. And, it will only physically be 18-bit color instead of
    24-bit but will increase the amount of data transferred by 50% (3 bytes per
    pixel instead of 2). Implementing this would have a side effect of making
    the one-off shared code (described above) more than one-off though.

[1]: https://github.com/Nkawu/TFT_22_ILI9225
[2]: http://www.hotmcu.com/22-176x220-tft-lcd-with-spi-interface-p-316.html

David Lechner (2):
  dt-bindings: Add binding for Ilitek ILI9225 display panels
  drm/tinydrm: add driver for ILI9225 panels

 .../devicetree/bindings/display/ilitek,ili9225.txt |  25 +
 MAINTAINERS                                        |   6 +
 drivers/gpu/drm/tinydrm/Kconfig                    |  10 +
 drivers/gpu/drm/tinydrm/Makefile                   |   1 +
 drivers/gpu/drm/tinydrm/ili9225.c                  | 547 +++++++++++++++++++++
 5 files changed, 589 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c

--
2.7.4

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

* [PATCH v1 0/2] DRM driver for ILI9225 display panels
@ 2017-11-08  3:52 ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2017-11-08  3:52 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, David Lechner, Rob Herring, linux-kernel

This is a new driver for ILI9225 based display panels.

There are a couple of things that stand out:

1.  Despite my best efforts, I could not find a name for this display[1], so I
    have made up a generic name for it. If someone recognizes this and has a
    name for it, please speak up. The best documentation I could find was
    here[2].

2.  There is quite a bit of one-off code copied from mipi-dbi.c. Based on the
    feedback from a previous patch series, I'm guessing that we don't want to
    modify mipi-dbi.c to accommodate the ILI9225 controller because the ILI9225
    controller does not use standard MIPI commands. ILI9225 has it's own
    non-standard command set, but other than that, it pretty much matches the
    generic mipi-dbi driver in all regards.

    Code that could be easily shared:

    a.  ili9225_buf_copy() is exactly the same as mipi_dbi_buf_copy()
        - ili9225_buf_copy() is used in ili9225_fb_dirty()
        - mipi_dbi_buf_copy() is static in mipi-dbi.c

    b.  ili9225_spi_cmd_max_speed() is exactly the same as
        mipi_dbi_spi_cmd_max_speed()
        - ili9225_spi_cmd_max_speed() is used in ili9225_command() below, so
          would not need to be copied if mipi_dbi_typec3_command() could be
          shared
        - mipi_dbi_spi_cmd_max_speed() is static in mipi-dbi.c

    c.  ili9225_command() is nearly the same as mipi_dbi_typec3_command()
        - a few unused lines were removed so I didn't have to copy even more
          code, but these would not be an issue if code was shared
        - cmd == ILI9225_WRITE_DATA_TO_GRAM instead of
          cmd == MIPI_DCS_WRITE_MEMORY_START

    d.  ili9225_spi_init() is nearly the same as mipi_dbi_spi_init()
        - a few unused lines were removed so I didn't have to copy even more
          code, but these would not be an issue if code was shared
        - mipi->command = ili9225_command instead of
          mipi->command = mipi_dbi_typec3_command
        - this function would not need to be copied if mipi_dbi_typec3_command()
          was modified to work with the ILI9225 command set too

    e.  ili9225_init() is nearly the same as mipi_dbi_init()
        - only difference is ili9225_fb_funcs instead of mipi_dbi_fb_funcs

3.  I haven't tried it, but I believe that it is possible to implement
    DRM_FORMAT_RGB888 directly instead of simulating DRM_FORMAT_XRGB8888.
    I don't know if there would be any real benefit to doing this. I am not
    familiar enough with userspace programs/libraries to know if this mode is
    universally used. And, it will only physically be 18-bit color instead of
    24-bit but will increase the amount of data transferred by 50% (3 bytes per
    pixel instead of 2). Implementing this would have a side effect of making
    the one-off shared code (described above) more than one-off though.

[1]: https://github.com/Nkawu/TFT_22_ILI9225
[2]: http://www.hotmcu.com/22-176x220-tft-lcd-with-spi-interface-p-316.html

David Lechner (2):
  dt-bindings: Add binding for Ilitek ILI9225 display panels
  drm/tinydrm: add driver for ILI9225 panels

 .../devicetree/bindings/display/ilitek,ili9225.txt |  25 +
 MAINTAINERS                                        |   6 +
 drivers/gpu/drm/tinydrm/Kconfig                    |  10 +
 drivers/gpu/drm/tinydrm/Makefile                   |   1 +
 drivers/gpu/drm/tinydrm/ili9225.c                  | 547 +++++++++++++++++++++
 5 files changed, 589 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c

--
2.7.4

_______________________________________________
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 v1 1/2] dt-bindings: Add binding for Ilitek ILI9225 display panels
  2017-11-08  3:52 ` David Lechner
@ 2017-11-08  3:52   ` David Lechner
  -1 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2017-11-08  3:52 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	linux-kernel

This adds a new binding for display panels that use an Ilitek ILI9225
controller.

The "generic,2.2in-176x220-ili9225-tft" device listed has no identification
markings whatsoever and an hour of googling turned up nothing, hence the use
of "generic".

An example of this nameless device can be found at:
https://github.com/Nkawu/TFT_22_ILI9225

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

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9225.txt b/Documentation/devicetree/bindings/display/ilitek,ili9225.txt
new file mode 100644
index 0000000..5839f56
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9225.txt
@@ -0,0 +1,25 @@
+Ilitek ILI9225 display panels
+
+This binding is for display panels using an Ilitek ILI9225 controller in SPI
+mode.
+
+Required properties:
+- compatible:	"generic,2.2in-176x220-ili9225-tft"
+- rs-gpios:	Register select signal
+- 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)
+
+Example:
+	display@0{
+		compatible = "generic,2.2in-176x220-ili9225-tft";
+		reg = <0>;
+		spi-max-frequency = <12000000>;
+		rs-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+		rotation = <270>;
+	};
-- 
2.7.4

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

* [PATCH v1 1/2] dt-bindings: Add binding for Ilitek ILI9225 display panels
@ 2017-11-08  3:52   ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2017-11-08  3:52 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, David Lechner, Rob Herring, linux-kernel

This adds a new binding for display panels that use an Ilitek ILI9225
controller.

The "generic,2.2in-176x220-ili9225-tft" device listed has no identification
markings whatsoever and an hour of googling turned up nothing, hence the use
of "generic".

An example of this nameless device can be found at:
https://github.com/Nkawu/TFT_22_ILI9225

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

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9225.txt b/Documentation/devicetree/bindings/display/ilitek,ili9225.txt
new file mode 100644
index 0000000..5839f56
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9225.txt
@@ -0,0 +1,25 @@
+Ilitek ILI9225 display panels
+
+This binding is for display panels using an Ilitek ILI9225 controller in SPI
+mode.
+
+Required properties:
+- compatible:	"generic,2.2in-176x220-ili9225-tft"
+- rs-gpios:	Register select signal
+- 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)
+
+Example:
+	display@0{
+		compatible = "generic,2.2in-176x220-ili9225-tft";
+		reg = <0>;
+		spi-max-frequency = <12000000>;
+		rs-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+		rotation = <270>;
+	};
-- 
2.7.4

_______________________________________________
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 v1 2/2] drm/tinydrm: add driver for ILI9225 panels
  2017-11-08  3:52 ` David Lechner
@ 2017-11-08  3:52   ` David Lechner
  -1 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2017-11-08  3:52 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: David Lechner, Noralf Trønnes, Rob Herring, Mark Rutland,
	linux-kernel

This adds a new driver for display panels based on the Ilitek ILI9225
controller.

This was developed for a no-name panel with a red PCB that is commonly
marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.

I really did try very hard to find a make and model for this panel, but
there doesn't seem to be one, so the best I can do is offer the picture
in the link above for identification.

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/ili9225.c | 547 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 564 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d77f22..72404f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4372,6 +4372,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 S:	Maintained
 F:	drivers/gpu/drm/tve200/
 
+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
+
 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 2e790e7..90c5bd5 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -12,6 +12,16 @@ menuconfig DRM_TINYDRM
 config TINYDRM_MIPI_DBI
 	tristate
 
+config TINYDRM_ILI9225
+	tristate "DRM support for ILI9225 display panels"
+	depends on DRM_TINYDRM && SPI
+	select TINYDRM_MIPI_DBI
+	help
+	  DRM driver for the following Ilitek ILI9225 panels:
+	  * No-name 2.2" color screen module
+
+	  If M is selected the module will be called ili9225.
+
 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 0c184bd..8aeee53 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
 obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
 
 # Displays
+obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.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/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
new file mode 100644
index 0000000..07e1b8b
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -0,0 +1,547 @@
+/*
+ * DRM driver for Ilitek ILI9225 panels
+ *
+ * Copyright 2017 David Lechner <david@lechnology.com>
+ *
+ * Lots of code copied from mipi-dbi.c
+ * Copyright 2016 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/delay.h>
+#include <linux/dma-buf.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+#include <video/mipi_display.h>
+
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+
+#define ILI9225_DRIVER_READ_CODE	0x00
+#define ILI9225_DRIVER_OUTPUT_CONTROL	0x01
+#define ILI9225_LCD_AC_DRIVING_CONTROL	0x02
+#define ILI9225_ENTRY_MODE		0x03
+#define ILI9225_DISPLAY_CONTROL_1	0x07
+#define ILI9225_BLANK_PERIOD_CONTROL_1	0x08
+#define ILI9225_FRAME_CYCLE_CONTROL	0x0b
+#define ILI9225_INTERFACE_CONTROL	0x0c
+#define ILI9225_OSCILLATION_CONTROL	0x0f
+#define ILI9225_POWER_CONTROL_1		0x10
+#define ILI9225_POWER_CONTROL_2		0x11
+#define ILI9225_POWER_CONTROL_3		0x12
+#define ILI9225_POWER_CONTROL_4		0x13
+#define ILI9225_POWER_CONTROL_5		0x14
+#define ILI9225_VCI_RECYCLING		0x15
+#define ILI9225_RAM_ADDRESS_SET_1	0x20
+#define ILI9225_RAM_ADDRESS_SET_2	0x21
+#define ILI9225_WRITE_DATA_TO_GRAM	0x22
+#define ILI9225_SOFTWARE_RESET		0x28
+#define ILI9225_GATE_SCAN_CONTROL	0x30
+#define ILI9225_VERTICAL_SCROLL_1	0x31
+#define ILI9225_VERTICAL_SCROLL_2	0x32
+#define ILI9225_VERTICAL_SCROLL_3	0x33
+#define ILI9225_PARTIAL_DRIVING_POS_1	0x34
+#define ILI9225_PARTIAL_DRIVING_POS_2	0x35
+#define ILI9225_HORIZ_WINDOW_ADDR_1	0x36
+#define ILI9225_HORIZ_WINDOW_ADDR_2	0x37
+#define ILI9225_VERT_WINDOW_ADDR_1	0x38
+#define ILI9225_VERT_WINDOW_ADDR_2	0x39
+#define ILI9225_GAMMA_CONTROL_1		0x50
+#define ILI9225_GAMMA_CONTROL_2		0x51
+#define ILI9225_GAMMA_CONTROL_3		0x52
+#define ILI9225_GAMMA_CONTROL_4		0x53
+#define ILI9225_GAMMA_CONTROL_5		0x54
+#define ILI9225_GAMMA_CONTROL_6		0x55
+#define ILI9225_GAMMA_CONTROL_7		0x56
+#define ILI9225_GAMMA_CONTROL_8		0x57
+#define ILI9225_GAMMA_CONTROL_9		0x58
+#define ILI9225_GAMMA_CONTROL_10	0x59
+
+static int ili9225_buf_copy(void *dst, struct drm_framebuffer *fb,
+			    struct drm_clip_rect *clip, bool swap)
+{
+	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
+	struct drm_format_name_buf format_name;
+	void *src = cma_obj->vaddr;
+	int ret = 0;
+
+	if (import_attach) {
+		ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
+					       DMA_FROM_DEVICE);
+		if (ret)
+			return ret;
+	}
+
+	switch (fb->format->format) {
+	case DRM_FORMAT_RGB565:
+		if (swap)
+			tinydrm_swab16(dst, src, fb, clip);
+		else
+			tinydrm_memcpy(dst, src, fb, clip);
+		break;
+	case DRM_FORMAT_XRGB8888:
+		tinydrm_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
+		break;
+	default:
+		dev_err_once(fb->dev->dev, "Format is not supported: %s\n",
+			     drm_get_format_name(fb->format->format,
+						 &format_name));
+		return -EINVAL;
+	}
+
+	if (import_attach)
+		ret = dma_buf_end_cpu_access(import_attach->dmabuf,
+					     DMA_FROM_DEVICE);
+	return ret;
+}
+
+static int ili9225_fb_dirty(struct drm_framebuffer *fb,
+			    struct drm_file *file_priv, unsigned int flags,
+			    unsigned int color, struct drm_clip_rect *clips,
+			    unsigned int num_clips)
+{
+	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	struct tinydrm_device *tdev = fb->dev->dev_private;
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	bool swap = mipi->swap_bytes;
+	struct drm_clip_rect clip;
+	u8 x_start, y_start;
+	u8 x1, x2, y1, y2;
+	int ret = 0;
+	bool full;
+	void *tr;
+
+	mutex_lock(&tdev->dirty_lock);
+
+	if (!mipi->enabled)
+		goto out_unlock;
+
+	/* fbdev can flush even when we're not interested */
+	if (tdev->pipe.plane.fb != fb)
+		goto out_unlock;
+
+	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
+				   fb->width, fb->height);
+
+	DRM_DEBUG("Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n", fb->base.id,
+		  clip.x1, clip.x2, clip.y1, clip.y2);
+
+	if (!mipi->dc || !full || swap ||
+	    fb->format->format == DRM_FORMAT_XRGB8888) {
+		tr = mipi->tx_buf;
+		ret = ili9225_buf_copy(mipi->tx_buf, fb, &clip, swap);
+		if (ret)
+			goto out_unlock;
+	} else {
+		tr = cma_obj->vaddr;
+	}
+
+	switch (mipi->rotation) {
+	default:
+		x1 = clip.x1;
+		x2 = clip.x2 - 1;
+		y1 = clip.y1;
+		y2 = clip.y2 - 1;
+		x_start = x1;
+		y_start = y1;
+		break;
+	case 90:
+		x1 = clip.y1;
+		x2 = clip.y2 - 1;
+		y1 = fb->width - clip.x2;
+		y2 = fb->width - clip.x1 - 1;
+		x_start = x1;
+		y_start = y2;
+		break;
+	case 180:
+		x1 = fb->width - clip.x2;
+		x2 = fb->width - clip.x1 - 1;
+		y1 = fb->height - clip.y2;
+		y2 = fb->height - clip.y1 - 1;
+		x_start = x2;
+		y_start = y2;
+		break;
+	case 270:
+		x1 = fb->height - clip.y2;
+		x2 = fb->height - clip.y1 - 1;
+		y1 = clip.x1;
+		y2 = clip.x2 - 1;
+		x_start = x2;
+		y_start = y1;
+		break;
+	}
+
+	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_1, 0x00, x2);
+	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_2, 0x00, x1);
+	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_1, 0x00, y2);
+	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_2, 0x00, y1);
+
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, x_start);
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, y_start);
+
+	ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr,
+				(clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
+
+out_unlock:
+	mutex_unlock(&tdev->dirty_lock);
+
+	if (ret)
+		dev_err_once(fb->dev->dev, "Failed to update display %d\n",
+			     ret);
+
+	return ret;
+}
+
+static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+	.dirty		= ili9225_fb_dirty,
+};
+
+static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
+				struct drm_crtc_state *crtc_state)
+{
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct drm_framebuffer *fb = pipe->plane.fb;
+	struct device *dev = tdev->drm->dev;
+	int ret;
+	u8 am_id;
+
+	DRM_DEBUG_KMS("\n");
+
+	mipi_dbi_hw_reset(mipi);
+
+	/*
+	 * There don't seem to be two example init sequences that match, so
+	 * using the one from the popular Arduino library for this display.
+	 * https://github.com/Nkawu/TFT_22_ILI9225/blob/master/src/TFT_22_ILI9225.cpp
+	 */
+
+	ret = mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x00, 0x00);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
+		return;
+	}
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x00, 0x00);
+
+	msleep(40);
+
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x18);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x61, 0x21);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x6f);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x49, 0x5f);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x08, 0x00);
+
+	msleep(10);
+
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x10, 0x3b);
+
+	msleep(50);
+
+	switch (mipi->rotation) {
+	default:
+		am_id = 0x30;
+		break;
+	case 90:
+		am_id = 0x18;
+		break;
+	case 180:
+		am_id = 0x00;
+		break;
+	case 270:
+		am_id = 0x28;
+		break;
+	}
+	mipi_dbi_command(mipi, ILI9225_DRIVER_OUTPUT_CONTROL, 0x01, 0x1c);
+	mipi_dbi_command(mipi, ILI9225_LCD_AC_DRIVING_CONTROL, 0x01, 0x00);
+	mipi_dbi_command(mipi, ILI9225_ENTRY_MODE, 0x10, am_id);
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_BLANK_PERIOD_CONTROL_1, 0x08, 0x08);
+	mipi_dbi_command(mipi, ILI9225_FRAME_CYCLE_CONTROL, 0x11, 0x00);
+	mipi_dbi_command(mipi, ILI9225_INTERFACE_CONTROL, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_OSCILLATION_CONTROL, 0x0d, 0x01);
+	mipi_dbi_command(mipi, ILI9225_VCI_RECYCLING, 0x00, 0x20);
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, 0x00);
+
+	mipi_dbi_command(mipi, ILI9225_GATE_SCAN_CONTROL, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_1, 0x00, 0xdb);
+	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_2, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_3, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_1, 0x00, 0xdb);
+	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_2, 0x00, 0x00);
+
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_1, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_2, 0x08, 0x08);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_3, 0x08, 0x0a);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_4, 0x00, 0x0a);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_5, 0x0a, 0x08);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_6, 0x08, 0x08);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_7, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_8, 0x0a, 0x00);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_9, 0x07, 0x10);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_10, 0x07, 0x10);
+
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x12);
+
+	msleep(50);
+
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x10, 0x17);
+
+	mipi->enabled = true;
+
+	if (fb)
+		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
+}
+
+static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+
+	DRM_DEBUG_KMS("\n");
+
+	if (!mipi->enabled)
+		return;
+
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
+	msleep(50);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x07);
+	msleep(50);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x0a, 0x02);
+
+	mipi->enabled = false;
+}
+
+static u32 ili9225_spi_cmd_max_speed(struct spi_device *spi, size_t len)
+{
+	if (len > 64)
+		return 0; /* use default */
+
+	return min_t(u32, 10000000, spi->max_speed_hz);
+}
+
+static int ili9225_command(struct mipi_dbi *mipi, u8 cmd, u8 *par, size_t num)
+{
+	struct spi_device *spi = mipi->spi;
+	unsigned int bpw = 8;
+	u32 speed_hz;
+	int ret;
+
+	gpiod_set_value_cansleep(mipi->dc, 0);
+	speed_hz = ili9225_spi_cmd_max_speed(spi, 1);
+	ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
+	if (ret || !num)
+		return ret;
+
+	if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
+		bpw = 16;
+
+	gpiod_set_value_cansleep(mipi->dc, 1);
+	speed_hz = ili9225_spi_cmd_max_speed(spi, num);
+
+	return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num);
+}
+
+static int ili9225_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
+			    struct gpio_desc *dc)
+{
+	size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
+	struct device *dev = &spi->dev;
+	int ret;
+
+	if (tx_size < 16) {
+		DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
+		return -EINVAL;
+	}
+
+	/*
+	 * Even though it's not the SPI device that does DMA (the master does),
+	 * the dma mask is necessary for the dma_alloc_wc() in
+	 * drm_gem_cma_create(). The dma_addr returned will be a physical
+	 * address which might be different from the bus address, but this is
+	 * not a problem since the address will not be used.
+	 * The virtual address is used in the transfer and the SPI core
+	 * re-maps it on the SPI master device using the DMA streaming API
+	 * (spi_map_buf()).
+	 */
+	if (!dev->coherent_dma_mask) {
+		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret) {
+			dev_warn(dev, "Failed to set dma mask %d\n", ret);
+			return ret;
+		}
+	}
+
+	mipi->spi = spi;
+
+	mipi->command = ili9225_command;
+	mipi->dc = dc;
+	if (tinydrm_machine_little_endian() &&
+	    !tinydrm_spi_bpw_supported(spi, 16))
+		mipi->swap_bytes = true;
+
+	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
+
+	return 0;
+}
+
+static const u32 ili9225_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static int ili9225_init(struct device *dev, struct mipi_dbi *mipi,
+			const struct drm_simple_display_pipe_funcs *pipe_funcs,
+			struct drm_driver *driver,
+			const struct drm_display_mode *mode,
+			unsigned int rotation)
+{
+	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
+	struct tinydrm_device *tdev = &mipi->tinydrm;
+	int ret;
+
+	if (!mipi->command)
+		return -EINVAL;
+
+	mutex_init(&mipi->cmdlock);
+
+	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
+	if (!mipi->tx_buf)
+		return -ENOMEM;
+
+	ret = devm_tinydrm_init(dev, tdev, &ili9225_fb_funcs, driver);
+	if (ret)
+		return ret;
+
+	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
+					DRM_MODE_CONNECTOR_VIRTUAL,
+					ili9225_formats,
+					ARRAY_SIZE(ili9225_formats), mode,
+					rotation);
+	if (ret)
+		return ret;
+
+	tdev->drm->mode_config.preferred_depth = 16;
+	mipi->rotation = rotation;
+
+	drm_mode_config_reset(tdev->drm);
+
+	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
+		      tdev->drm->mode_config.preferred_depth, rotation);
+
+	return 0;
+}
+
+static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
+	.enable		= ili9225_pipe_enable,
+	.disable	= ili9225_pipe_disable,
+	.update		= tinydrm_display_pipe_update,
+	.prepare_fb	= tinydrm_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode ili9225_mode = {
+	TINYDRM_MODE(176, 220, 35, 44),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
+
+static struct drm_driver ili9225_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
+				  DRIVER_ATOMIC,
+	.fops			= &ili9225_fops,
+	TINYDRM_GEM_DRIVER_OPS,
+	.lastclose		= tinydrm_lastclose,
+	.name			= "ili9225",
+	.desc			= "Ilitek ILI9225",
+	.date			= "20171106",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static const struct of_device_id ili9225_of_match[] = {
+	{ .compatible = "generic,2.2in-176x220-ili9225-tft" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ili9225_of_match);
+
+static const struct spi_device_id ili9225_id[] = {
+	{ "2.2in-176x220-ili9225-tft", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, ili9225_id);
+
+static int ili9225_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dbi *mipi;
+	struct gpio_desc *rs;
+	u32 rotation = 0;
+	int ret;
+
+	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	if (!mipi)
+		return -ENOMEM;
+
+	mipi->reset = devm_gpiod_get(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);
+	}
+
+	rs = devm_gpiod_get(dev, "rs", GPIOD_OUT_LOW);
+	if (IS_ERR(rs)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'rs'\n");
+		return PTR_ERR(rs);
+	}
+
+	device_property_read_u32(dev, "rotation", &rotation);
+
+	ret = ili9225_spi_init(spi, mipi, rs);
+	if (ret)
+		return ret;
+
+	ret = ili9225_init(&spi->dev, mipi, &ili9225_pipe_funcs,
+			   &ili9225_driver, &ili9225_mode, rotation);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, mipi);
+
+	return devm_tinydrm_register(&mipi->tinydrm);
+}
+
+static void ili9225_shutdown(struct spi_device *spi)
+{
+	struct mipi_dbi *mipi = spi_get_drvdata(spi);
+
+	tinydrm_shutdown(&mipi->tinydrm);
+}
+
+static struct spi_driver ili9225_spi_driver = {
+	.driver = {
+		.name = "ili9225",
+		.owner = THIS_MODULE,
+		.of_match_table = ili9225_of_match,
+	},
+	.id_table = ili9225_id,
+	.probe = ili9225_probe,
+	.shutdown = ili9225_shutdown,
+};
+module_spi_driver(ili9225_spi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9225 DRM driver");
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_LICENSE("GPL");
-- 
2.7.4

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

* [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-11-08  3:52   ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2017-11-08  3:52 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Mark Rutland, David Lechner, Rob Herring, linux-kernel

This adds a new driver for display panels based on the Ilitek ILI9225
controller.

This was developed for a no-name panel with a red PCB that is commonly
marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.

I really did try very hard to find a make and model for this panel, but
there doesn't seem to be one, so the best I can do is offer the picture
in the link above for identification.

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/ili9225.c | 547 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 564 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d77f22..72404f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4372,6 +4372,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 S:	Maintained
 F:	drivers/gpu/drm/tve200/
 
+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
+
 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 2e790e7..90c5bd5 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -12,6 +12,16 @@ menuconfig DRM_TINYDRM
 config TINYDRM_MIPI_DBI
 	tristate
 
+config TINYDRM_ILI9225
+	tristate "DRM support for ILI9225 display panels"
+	depends on DRM_TINYDRM && SPI
+	select TINYDRM_MIPI_DBI
+	help
+	  DRM driver for the following Ilitek ILI9225 panels:
+	  * No-name 2.2" color screen module
+
+	  If M is selected the module will be called ili9225.
+
 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 0c184bd..8aeee53 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
 obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
 
 # Displays
+obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.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/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
new file mode 100644
index 0000000..07e1b8b
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -0,0 +1,547 @@
+/*
+ * DRM driver for Ilitek ILI9225 panels
+ *
+ * Copyright 2017 David Lechner <david@lechnology.com>
+ *
+ * Lots of code copied from mipi-dbi.c
+ * Copyright 2016 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/delay.h>
+#include <linux/dma-buf.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+#include <video/mipi_display.h>
+
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+
+#define ILI9225_DRIVER_READ_CODE	0x00
+#define ILI9225_DRIVER_OUTPUT_CONTROL	0x01
+#define ILI9225_LCD_AC_DRIVING_CONTROL	0x02
+#define ILI9225_ENTRY_MODE		0x03
+#define ILI9225_DISPLAY_CONTROL_1	0x07
+#define ILI9225_BLANK_PERIOD_CONTROL_1	0x08
+#define ILI9225_FRAME_CYCLE_CONTROL	0x0b
+#define ILI9225_INTERFACE_CONTROL	0x0c
+#define ILI9225_OSCILLATION_CONTROL	0x0f
+#define ILI9225_POWER_CONTROL_1		0x10
+#define ILI9225_POWER_CONTROL_2		0x11
+#define ILI9225_POWER_CONTROL_3		0x12
+#define ILI9225_POWER_CONTROL_4		0x13
+#define ILI9225_POWER_CONTROL_5		0x14
+#define ILI9225_VCI_RECYCLING		0x15
+#define ILI9225_RAM_ADDRESS_SET_1	0x20
+#define ILI9225_RAM_ADDRESS_SET_2	0x21
+#define ILI9225_WRITE_DATA_TO_GRAM	0x22
+#define ILI9225_SOFTWARE_RESET		0x28
+#define ILI9225_GATE_SCAN_CONTROL	0x30
+#define ILI9225_VERTICAL_SCROLL_1	0x31
+#define ILI9225_VERTICAL_SCROLL_2	0x32
+#define ILI9225_VERTICAL_SCROLL_3	0x33
+#define ILI9225_PARTIAL_DRIVING_POS_1	0x34
+#define ILI9225_PARTIAL_DRIVING_POS_2	0x35
+#define ILI9225_HORIZ_WINDOW_ADDR_1	0x36
+#define ILI9225_HORIZ_WINDOW_ADDR_2	0x37
+#define ILI9225_VERT_WINDOW_ADDR_1	0x38
+#define ILI9225_VERT_WINDOW_ADDR_2	0x39
+#define ILI9225_GAMMA_CONTROL_1		0x50
+#define ILI9225_GAMMA_CONTROL_2		0x51
+#define ILI9225_GAMMA_CONTROL_3		0x52
+#define ILI9225_GAMMA_CONTROL_4		0x53
+#define ILI9225_GAMMA_CONTROL_5		0x54
+#define ILI9225_GAMMA_CONTROL_6		0x55
+#define ILI9225_GAMMA_CONTROL_7		0x56
+#define ILI9225_GAMMA_CONTROL_8		0x57
+#define ILI9225_GAMMA_CONTROL_9		0x58
+#define ILI9225_GAMMA_CONTROL_10	0x59
+
+static int ili9225_buf_copy(void *dst, struct drm_framebuffer *fb,
+			    struct drm_clip_rect *clip, bool swap)
+{
+	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
+	struct drm_format_name_buf format_name;
+	void *src = cma_obj->vaddr;
+	int ret = 0;
+
+	if (import_attach) {
+		ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
+					       DMA_FROM_DEVICE);
+		if (ret)
+			return ret;
+	}
+
+	switch (fb->format->format) {
+	case DRM_FORMAT_RGB565:
+		if (swap)
+			tinydrm_swab16(dst, src, fb, clip);
+		else
+			tinydrm_memcpy(dst, src, fb, clip);
+		break;
+	case DRM_FORMAT_XRGB8888:
+		tinydrm_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
+		break;
+	default:
+		dev_err_once(fb->dev->dev, "Format is not supported: %s\n",
+			     drm_get_format_name(fb->format->format,
+						 &format_name));
+		return -EINVAL;
+	}
+
+	if (import_attach)
+		ret = dma_buf_end_cpu_access(import_attach->dmabuf,
+					     DMA_FROM_DEVICE);
+	return ret;
+}
+
+static int ili9225_fb_dirty(struct drm_framebuffer *fb,
+			    struct drm_file *file_priv, unsigned int flags,
+			    unsigned int color, struct drm_clip_rect *clips,
+			    unsigned int num_clips)
+{
+	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	struct tinydrm_device *tdev = fb->dev->dev_private;
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	bool swap = mipi->swap_bytes;
+	struct drm_clip_rect clip;
+	u8 x_start, y_start;
+	u8 x1, x2, y1, y2;
+	int ret = 0;
+	bool full;
+	void *tr;
+
+	mutex_lock(&tdev->dirty_lock);
+
+	if (!mipi->enabled)
+		goto out_unlock;
+
+	/* fbdev can flush even when we're not interested */
+	if (tdev->pipe.plane.fb != fb)
+		goto out_unlock;
+
+	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
+				   fb->width, fb->height);
+
+	DRM_DEBUG("Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n", fb->base.id,
+		  clip.x1, clip.x2, clip.y1, clip.y2);
+
+	if (!mipi->dc || !full || swap ||
+	    fb->format->format == DRM_FORMAT_XRGB8888) {
+		tr = mipi->tx_buf;
+		ret = ili9225_buf_copy(mipi->tx_buf, fb, &clip, swap);
+		if (ret)
+			goto out_unlock;
+	} else {
+		tr = cma_obj->vaddr;
+	}
+
+	switch (mipi->rotation) {
+	default:
+		x1 = clip.x1;
+		x2 = clip.x2 - 1;
+		y1 = clip.y1;
+		y2 = clip.y2 - 1;
+		x_start = x1;
+		y_start = y1;
+		break;
+	case 90:
+		x1 = clip.y1;
+		x2 = clip.y2 - 1;
+		y1 = fb->width - clip.x2;
+		y2 = fb->width - clip.x1 - 1;
+		x_start = x1;
+		y_start = y2;
+		break;
+	case 180:
+		x1 = fb->width - clip.x2;
+		x2 = fb->width - clip.x1 - 1;
+		y1 = fb->height - clip.y2;
+		y2 = fb->height - clip.y1 - 1;
+		x_start = x2;
+		y_start = y2;
+		break;
+	case 270:
+		x1 = fb->height - clip.y2;
+		x2 = fb->height - clip.y1 - 1;
+		y1 = clip.x1;
+		y2 = clip.x2 - 1;
+		x_start = x2;
+		y_start = y1;
+		break;
+	}
+
+	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_1, 0x00, x2);
+	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_2, 0x00, x1);
+	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_1, 0x00, y2);
+	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_2, 0x00, y1);
+
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, x_start);
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, y_start);
+
+	ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr,
+				(clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
+
+out_unlock:
+	mutex_unlock(&tdev->dirty_lock);
+
+	if (ret)
+		dev_err_once(fb->dev->dev, "Failed to update display %d\n",
+			     ret);
+
+	return ret;
+}
+
+static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+	.dirty		= ili9225_fb_dirty,
+};
+
+static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
+				struct drm_crtc_state *crtc_state)
+{
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct drm_framebuffer *fb = pipe->plane.fb;
+	struct device *dev = tdev->drm->dev;
+	int ret;
+	u8 am_id;
+
+	DRM_DEBUG_KMS("\n");
+
+	mipi_dbi_hw_reset(mipi);
+
+	/*
+	 * There don't seem to be two example init sequences that match, so
+	 * using the one from the popular Arduino library for this display.
+	 * https://github.com/Nkawu/TFT_22_ILI9225/blob/master/src/TFT_22_ILI9225.cpp
+	 */
+
+	ret = mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x00, 0x00);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
+		return;
+	}
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x00, 0x00);
+
+	msleep(40);
+
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x18);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x61, 0x21);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x6f);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x49, 0x5f);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x08, 0x00);
+
+	msleep(10);
+
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x10, 0x3b);
+
+	msleep(50);
+
+	switch (mipi->rotation) {
+	default:
+		am_id = 0x30;
+		break;
+	case 90:
+		am_id = 0x18;
+		break;
+	case 180:
+		am_id = 0x00;
+		break;
+	case 270:
+		am_id = 0x28;
+		break;
+	}
+	mipi_dbi_command(mipi, ILI9225_DRIVER_OUTPUT_CONTROL, 0x01, 0x1c);
+	mipi_dbi_command(mipi, ILI9225_LCD_AC_DRIVING_CONTROL, 0x01, 0x00);
+	mipi_dbi_command(mipi, ILI9225_ENTRY_MODE, 0x10, am_id);
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_BLANK_PERIOD_CONTROL_1, 0x08, 0x08);
+	mipi_dbi_command(mipi, ILI9225_FRAME_CYCLE_CONTROL, 0x11, 0x00);
+	mipi_dbi_command(mipi, ILI9225_INTERFACE_CONTROL, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_OSCILLATION_CONTROL, 0x0d, 0x01);
+	mipi_dbi_command(mipi, ILI9225_VCI_RECYCLING, 0x00, 0x20);
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, 0x00);
+
+	mipi_dbi_command(mipi, ILI9225_GATE_SCAN_CONTROL, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_1, 0x00, 0xdb);
+	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_2, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_3, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_1, 0x00, 0xdb);
+	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_2, 0x00, 0x00);
+
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_1, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_2, 0x08, 0x08);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_3, 0x08, 0x0a);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_4, 0x00, 0x0a);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_5, 0x0a, 0x08);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_6, 0x08, 0x08);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_7, 0x00, 0x00);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_8, 0x0a, 0x00);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_9, 0x07, 0x10);
+	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_10, 0x07, 0x10);
+
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x12);
+
+	msleep(50);
+
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x10, 0x17);
+
+	mipi->enabled = true;
+
+	if (fb)
+		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
+}
+
+static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+
+	DRM_DEBUG_KMS("\n");
+
+	if (!mipi->enabled)
+		return;
+
+	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
+	msleep(50);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x07);
+	msleep(50);
+	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x0a, 0x02);
+
+	mipi->enabled = false;
+}
+
+static u32 ili9225_spi_cmd_max_speed(struct spi_device *spi, size_t len)
+{
+	if (len > 64)
+		return 0; /* use default */
+
+	return min_t(u32, 10000000, spi->max_speed_hz);
+}
+
+static int ili9225_command(struct mipi_dbi *mipi, u8 cmd, u8 *par, size_t num)
+{
+	struct spi_device *spi = mipi->spi;
+	unsigned int bpw = 8;
+	u32 speed_hz;
+	int ret;
+
+	gpiod_set_value_cansleep(mipi->dc, 0);
+	speed_hz = ili9225_spi_cmd_max_speed(spi, 1);
+	ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
+	if (ret || !num)
+		return ret;
+
+	if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
+		bpw = 16;
+
+	gpiod_set_value_cansleep(mipi->dc, 1);
+	speed_hz = ili9225_spi_cmd_max_speed(spi, num);
+
+	return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num);
+}
+
+static int ili9225_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
+			    struct gpio_desc *dc)
+{
+	size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
+	struct device *dev = &spi->dev;
+	int ret;
+
+	if (tx_size < 16) {
+		DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
+		return -EINVAL;
+	}
+
+	/*
+	 * Even though it's not the SPI device that does DMA (the master does),
+	 * the dma mask is necessary for the dma_alloc_wc() in
+	 * drm_gem_cma_create(). The dma_addr returned will be a physical
+	 * address which might be different from the bus address, but this is
+	 * not a problem since the address will not be used.
+	 * The virtual address is used in the transfer and the SPI core
+	 * re-maps it on the SPI master device using the DMA streaming API
+	 * (spi_map_buf()).
+	 */
+	if (!dev->coherent_dma_mask) {
+		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret) {
+			dev_warn(dev, "Failed to set dma mask %d\n", ret);
+			return ret;
+		}
+	}
+
+	mipi->spi = spi;
+
+	mipi->command = ili9225_command;
+	mipi->dc = dc;
+	if (tinydrm_machine_little_endian() &&
+	    !tinydrm_spi_bpw_supported(spi, 16))
+		mipi->swap_bytes = true;
+
+	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
+
+	return 0;
+}
+
+static const u32 ili9225_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static int ili9225_init(struct device *dev, struct mipi_dbi *mipi,
+			const struct drm_simple_display_pipe_funcs *pipe_funcs,
+			struct drm_driver *driver,
+			const struct drm_display_mode *mode,
+			unsigned int rotation)
+{
+	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
+	struct tinydrm_device *tdev = &mipi->tinydrm;
+	int ret;
+
+	if (!mipi->command)
+		return -EINVAL;
+
+	mutex_init(&mipi->cmdlock);
+
+	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
+	if (!mipi->tx_buf)
+		return -ENOMEM;
+
+	ret = devm_tinydrm_init(dev, tdev, &ili9225_fb_funcs, driver);
+	if (ret)
+		return ret;
+
+	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
+					DRM_MODE_CONNECTOR_VIRTUAL,
+					ili9225_formats,
+					ARRAY_SIZE(ili9225_formats), mode,
+					rotation);
+	if (ret)
+		return ret;
+
+	tdev->drm->mode_config.preferred_depth = 16;
+	mipi->rotation = rotation;
+
+	drm_mode_config_reset(tdev->drm);
+
+	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
+		      tdev->drm->mode_config.preferred_depth, rotation);
+
+	return 0;
+}
+
+static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
+	.enable		= ili9225_pipe_enable,
+	.disable	= ili9225_pipe_disable,
+	.update		= tinydrm_display_pipe_update,
+	.prepare_fb	= tinydrm_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode ili9225_mode = {
+	TINYDRM_MODE(176, 220, 35, 44),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
+
+static struct drm_driver ili9225_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
+				  DRIVER_ATOMIC,
+	.fops			= &ili9225_fops,
+	TINYDRM_GEM_DRIVER_OPS,
+	.lastclose		= tinydrm_lastclose,
+	.name			= "ili9225",
+	.desc			= "Ilitek ILI9225",
+	.date			= "20171106",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static const struct of_device_id ili9225_of_match[] = {
+	{ .compatible = "generic,2.2in-176x220-ili9225-tft" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ili9225_of_match);
+
+static const struct spi_device_id ili9225_id[] = {
+	{ "2.2in-176x220-ili9225-tft", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, ili9225_id);
+
+static int ili9225_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dbi *mipi;
+	struct gpio_desc *rs;
+	u32 rotation = 0;
+	int ret;
+
+	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	if (!mipi)
+		return -ENOMEM;
+
+	mipi->reset = devm_gpiod_get(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);
+	}
+
+	rs = devm_gpiod_get(dev, "rs", GPIOD_OUT_LOW);
+	if (IS_ERR(rs)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'rs'\n");
+		return PTR_ERR(rs);
+	}
+
+	device_property_read_u32(dev, "rotation", &rotation);
+
+	ret = ili9225_spi_init(spi, mipi, rs);
+	if (ret)
+		return ret;
+
+	ret = ili9225_init(&spi->dev, mipi, &ili9225_pipe_funcs,
+			   &ili9225_driver, &ili9225_mode, rotation);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, mipi);
+
+	return devm_tinydrm_register(&mipi->tinydrm);
+}
+
+static void ili9225_shutdown(struct spi_device *spi)
+{
+	struct mipi_dbi *mipi = spi_get_drvdata(spi);
+
+	tinydrm_shutdown(&mipi->tinydrm);
+}
+
+static struct spi_driver ili9225_spi_driver = {
+	.driver = {
+		.name = "ili9225",
+		.owner = THIS_MODULE,
+		.of_match_table = ili9225_of_match,
+	},
+	.id_table = ili9225_id,
+	.probe = ili9225_probe,
+	.shutdown = ili9225_shutdown,
+};
+module_spi_driver(ili9225_spi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9225 DRM driver");
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_LICENSE("GPL");
-- 
2.7.4

_______________________________________________
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 v1 0/2] DRM driver for ILI9225 display panels
@ 2017-11-10 12:33   ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2017-11-10 12:33 UTC (permalink / raw)
  To: David Lechner, dri-devel, devicetree
  Cc: Rob Herring, Mark Rutland, linux-kernel

Hi David, nice to see a new driver.

Den 08.11.2017 04.52, skrev David Lechner:
> This is a new driver for ILI9225 based display panels.
>
> There are a couple of things that stand out:
>
> 1.  Despite my best efforts, I could not find a name for this display[1], so I
>      have made up a generic name for it. If someone recognizes this and has a
>      name for it, please speak up. The best documentation I could find was
>      here[2].
>
> 2.  There is quite a bit of one-off code copied from mipi-dbi.c. Based on the
>      feedback from a previous patch series, I'm guessing that we don't want to
>      modify mipi-dbi.c to accommodate the ILI9225 controller because the ILI9225
>      controller does not use standard MIPI commands. ILI9225 has it's own
>      non-standard command set, but other than that, it pretty much matches the
>      generic mipi-dbi driver in all regards.

Yes you're right, I don't want non MIPI code in mipi-dbi, but we can
try and make it flexible for usage with DBI like interfaces.

>      Code that could be easily shared:
>
>      a.  ili9225_buf_copy() is exactly the same as mipi_dbi_buf_copy()
>          - ili9225_buf_copy() is used in ili9225_fb_dirty()
>          - mipi_dbi_buf_copy() is static in mipi-dbi.c

You can export mipi_dbi_buf_copy().

>      b.  ili9225_spi_cmd_max_speed() is exactly the same as
>          mipi_dbi_spi_cmd_max_speed()
>          - ili9225_spi_cmd_max_speed() is used in ili9225_command() below, so
>            would not need to be copied if mipi_dbi_typec3_command() could be
>            shared
>          - mipi_dbi_spi_cmd_max_speed() is static in mipi-dbi.c

Same here, you can export the function.

>      c.  ili9225_command() is nearly the same as mipi_dbi_typec3_command()
>          - a few unused lines were removed so I didn't have to copy even more
>            code, but these would not be an issue if code was shared
>          - cmd == ILI9225_WRITE_DATA_TO_GRAM instead of
>            cmd == MIPI_DCS_WRITE_MEMORY_START
>
>      d.  ili9225_spi_init() is nearly the same as mipi_dbi_spi_init()
>          - a few unused lines were removed so I didn't have to copy even more
>            code, but these would not be an issue if code was shared
>          - mipi->command = ili9225_command instead of
>            mipi->command = mipi_dbi_typec3_command
>          - this function would not need to be copied if mipi_dbi_typec3_command()
>            was modified to work with the ILI9225 command set too

Looks like you can use mipi_dbi_spi_init() and reassign afterwards:
     mipi_dbi_spi_init();
     mipi->command = ili9225_command;

>      e.  ili9225_init() is nearly the same as mipi_dbi_init()
>          - only difference is ili9225_fb_funcs instead of mipi_dbi_fb_funcs

When we get framebuffer flushing trough atomic modeset, we'll use a generic
fb_dirty and at that point it will be possible to use mipi_dbi_init().
The actual flushing will happen in the update callback I guess.

> 3.  I haven't tried it, but I believe that it is possible to implement
>      DRM_FORMAT_RGB888 directly instead of simulating DRM_FORMAT_XRGB8888.
>      I don't know if there would be any real benefit to doing this. I am not
>      familiar enough with userspace programs/libraries to know if this mode is
>      universally used. And, it will only physically be 18-bit color instead of
>      24-bit but will increase the amount of data transferred by 50% (3 bytes per
>      pixel instead of 2). Implementing this would have a side effect of making
>      the one-off shared code (described above) more than one-off though.

I have tried it on a display and I couldn't really tell the difference
between 16-bit and 18-bit colors, with X windows at least. And as you
say it's terrible for the framerate.

I'll look closer through the driver later today and see if I have more
comments.

Side note:
This controller can also do something called 24-bit 4 wire SPI. It uses a
Start byte the same way that the (only) SPI mode on ILI9320 and ILI9325 do.
I did a test implementation for ILI9325 where I used regmap as the register
abstraction: https://github.com/notro/tinydrm/blob/master/tinydrm-ili9325.c

Noralf.

> [1]: https://github.com/Nkawu/TFT_22_ILI9225
> [2]: http://www.hotmcu.com/22-176x220-tft-lcd-with-spi-interface-p-316.html
>
> David Lechner (2):
>    dt-bindings: Add binding for Ilitek ILI9225 display panels
>    drm/tinydrm: add driver for ILI9225 panels
>
>   .../devicetree/bindings/display/ilitek,ili9225.txt |  25 +
>   MAINTAINERS                                        |   6 +
>   drivers/gpu/drm/tinydrm/Kconfig                    |  10 +
>   drivers/gpu/drm/tinydrm/Makefile                   |   1 +
>   drivers/gpu/drm/tinydrm/ili9225.c                  | 547 +++++++++++++++++++++
>   5 files changed, 589 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>   create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c
>
> --
> 2.7.4
>

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

* Re: [PATCH v1 0/2] DRM driver for ILI9225 display panels
@ 2017-11-10 12:33   ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2017-11-10 12:33 UTC (permalink / raw)
  To: David Lechner, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Rob Herring, Mark Rutland, linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi David, nice to see a new driver.

Den 08.11.2017 04.52, skrev David Lechner:
> This is a new driver for ILI9225 based display panels.
>
> There are a couple of things that stand out:
>
> 1.  Despite my best efforts, I could not find a name for this display[1], so I
>      have made up a generic name for it. If someone recognizes this and has a
>      name for it, please speak up. The best documentation I could find was
>      here[2].
>
> 2.  There is quite a bit of one-off code copied from mipi-dbi.c. Based on the
>      feedback from a previous patch series, I'm guessing that we don't want to
>      modify mipi-dbi.c to accommodate the ILI9225 controller because the ILI9225
>      controller does not use standard MIPI commands. ILI9225 has it's own
>      non-standard command set, but other than that, it pretty much matches the
>      generic mipi-dbi driver in all regards.

Yes you're right, I don't want non MIPI code in mipi-dbi, but we can
try and make it flexible for usage with DBI like interfaces.

>      Code that could be easily shared:
>
>      a.  ili9225_buf_copy() is exactly the same as mipi_dbi_buf_copy()
>          - ili9225_buf_copy() is used in ili9225_fb_dirty()
>          - mipi_dbi_buf_copy() is static in mipi-dbi.c

You can export mipi_dbi_buf_copy().

>      b.  ili9225_spi_cmd_max_speed() is exactly the same as
>          mipi_dbi_spi_cmd_max_speed()
>          - ili9225_spi_cmd_max_speed() is used in ili9225_command() below, so
>            would not need to be copied if mipi_dbi_typec3_command() could be
>            shared
>          - mipi_dbi_spi_cmd_max_speed() is static in mipi-dbi.c

Same here, you can export the function.

>      c.  ili9225_command() is nearly the same as mipi_dbi_typec3_command()
>          - a few unused lines were removed so I didn't have to copy even more
>            code, but these would not be an issue if code was shared
>          - cmd == ILI9225_WRITE_DATA_TO_GRAM instead of
>            cmd == MIPI_DCS_WRITE_MEMORY_START
>
>      d.  ili9225_spi_init() is nearly the same as mipi_dbi_spi_init()
>          - a few unused lines were removed so I didn't have to copy even more
>            code, but these would not be an issue if code was shared
>          - mipi->command = ili9225_command instead of
>            mipi->command = mipi_dbi_typec3_command
>          - this function would not need to be copied if mipi_dbi_typec3_command()
>            was modified to work with the ILI9225 command set too

Looks like you can use mipi_dbi_spi_init() and reassign afterwards:
     mipi_dbi_spi_init();
     mipi->command = ili9225_command;

>      e.  ili9225_init() is nearly the same as mipi_dbi_init()
>          - only difference is ili9225_fb_funcs instead of mipi_dbi_fb_funcs

When we get framebuffer flushing trough atomic modeset, we'll use a generic
fb_dirty and at that point it will be possible to use mipi_dbi_init().
The actual flushing will happen in the update callback I guess.

> 3.  I haven't tried it, but I believe that it is possible to implement
>      DRM_FORMAT_RGB888 directly instead of simulating DRM_FORMAT_XRGB8888.
>      I don't know if there would be any real benefit to doing this. I am not
>      familiar enough with userspace programs/libraries to know if this mode is
>      universally used. And, it will only physically be 18-bit color instead of
>      24-bit but will increase the amount of data transferred by 50% (3 bytes per
>      pixel instead of 2). Implementing this would have a side effect of making
>      the one-off shared code (described above) more than one-off though.

I have tried it on a display and I couldn't really tell the difference
between 16-bit and 18-bit colors, with X windows at least. And as you
say it's terrible for the framerate.

I'll look closer through the driver later today and see if I have more
comments.

Side note:
This controller can also do something called 24-bit 4 wire SPI. It uses a
Start byte the same way that the (only) SPI mode on ILI9320 and ILI9325 do.
I did a test implementation for ILI9325 where I used regmap as the register
abstraction: https://github.com/notro/tinydrm/blob/master/tinydrm-ili9325.c

Noralf.

> [1]: https://github.com/Nkawu/TFT_22_ILI9225
> [2]: http://www.hotmcu.com/22-176x220-tft-lcd-with-spi-interface-p-316.html
>
> David Lechner (2):
>    dt-bindings: Add binding for Ilitek ILI9225 display panels
>    drm/tinydrm: add driver for ILI9225 panels
>
>   .../devicetree/bindings/display/ilitek,ili9225.txt |  25 +
>   MAINTAINERS                                        |   6 +
>   drivers/gpu/drm/tinydrm/Kconfig                    |  10 +
>   drivers/gpu/drm/tinydrm/Makefile                   |   1 +
>   drivers/gpu/drm/tinydrm/ili9225.c                  | 547 +++++++++++++++++++++
>   5 files changed, 589 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>   create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c
>
> --
> 2.7.4
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-11-10 18:44     ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2017-11-10 18:44 UTC (permalink / raw)
  To: David Lechner, dri-devel, devicetree
  Cc: Rob Herring, Mark Rutland, linux-kernel


Den 08.11.2017 04.52, skrev David Lechner:
> This adds a new driver for display panels based on the Ilitek ILI9225
> controller.
>
> This was developed for a no-name panel with a red PCB that is commonly
> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>
> I really did try very hard to find a make and model for this panel, but
> there doesn't seem to be one, so the best I can do is offer the picture
> in the link above for identification.
>
> 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/ili9225.c | 547 ++++++++++++++++++++++++++++++++++++++
>   4 files changed, 564 insertions(+)
>   create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0d77f22..72404f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4372,6 +4372,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
>   S:	Maintained
>   F:	drivers/gpu/drm/tve200/
>   
> +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
> +
>   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 2e790e7..90c5bd5 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -12,6 +12,16 @@ menuconfig DRM_TINYDRM
>   config TINYDRM_MIPI_DBI
>   	tristate
>   
> +config TINYDRM_ILI9225
> +	tristate "DRM support for ILI9225 display panels"
> +	depends on DRM_TINYDRM && SPI
> +	select TINYDRM_MIPI_DBI
> +	help
> +	  DRM driver for the following Ilitek ILI9225 panels:
> +	  * No-name 2.2" color screen module
> +
> +	  If M is selected the module will be called ili9225.
> +
>   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 0c184bd..8aeee53 100644
> --- a/drivers/gpu/drm/tinydrm/Makefile
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
>   obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
>   
>   # Displays
> +obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.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/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
> new file mode 100644
> index 0000000..07e1b8b
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
> @@ -0,0 +1,547 @@
> +/*
> + * DRM driver for Ilitek ILI9225 panels
> + *
> + * Copyright 2017 David Lechner <david@lechnology.com>
> + *
> + * Lots of code copied from mipi-dbi.c
> + * Copyright 2016 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/dma-buf.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/spi/spi.h>
> +#include <video/mipi_display.h>
> +
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/tinydrm/mipi-dbi.h>
> +#include <drm/tinydrm/tinydrm-helpers.h>
> +
> +#define ILI9225_DRIVER_READ_CODE	0x00
> +#define ILI9225_DRIVER_OUTPUT_CONTROL	0x01
> +#define ILI9225_LCD_AC_DRIVING_CONTROL	0x02
> +#define ILI9225_ENTRY_MODE		0x03
> +#define ILI9225_DISPLAY_CONTROL_1	0x07
> +#define ILI9225_BLANK_PERIOD_CONTROL_1	0x08
> +#define ILI9225_FRAME_CYCLE_CONTROL	0x0b
> +#define ILI9225_INTERFACE_CONTROL	0x0c
> +#define ILI9225_OSCILLATION_CONTROL	0x0f
> +#define ILI9225_POWER_CONTROL_1		0x10
> +#define ILI9225_POWER_CONTROL_2		0x11
> +#define ILI9225_POWER_CONTROL_3		0x12
> +#define ILI9225_POWER_CONTROL_4		0x13
> +#define ILI9225_POWER_CONTROL_5		0x14
> +#define ILI9225_VCI_RECYCLING		0x15
> +#define ILI9225_RAM_ADDRESS_SET_1	0x20
> +#define ILI9225_RAM_ADDRESS_SET_2	0x21
> +#define ILI9225_WRITE_DATA_TO_GRAM	0x22
> +#define ILI9225_SOFTWARE_RESET		0x28
> +#define ILI9225_GATE_SCAN_CONTROL	0x30
> +#define ILI9225_VERTICAL_SCROLL_1	0x31
> +#define ILI9225_VERTICAL_SCROLL_2	0x32
> +#define ILI9225_VERTICAL_SCROLL_3	0x33
> +#define ILI9225_PARTIAL_DRIVING_POS_1	0x34
> +#define ILI9225_PARTIAL_DRIVING_POS_2	0x35
> +#define ILI9225_HORIZ_WINDOW_ADDR_1	0x36
> +#define ILI9225_HORIZ_WINDOW_ADDR_2	0x37
> +#define ILI9225_VERT_WINDOW_ADDR_1	0x38
> +#define ILI9225_VERT_WINDOW_ADDR_2	0x39
> +#define ILI9225_GAMMA_CONTROL_1		0x50
> +#define ILI9225_GAMMA_CONTROL_2		0x51
> +#define ILI9225_GAMMA_CONTROL_3		0x52
> +#define ILI9225_GAMMA_CONTROL_4		0x53
> +#define ILI9225_GAMMA_CONTROL_5		0x54
> +#define ILI9225_GAMMA_CONTROL_6		0x55
> +#define ILI9225_GAMMA_CONTROL_7		0x56
> +#define ILI9225_GAMMA_CONTROL_8		0x57
> +#define ILI9225_GAMMA_CONTROL_9		0x58
> +#define ILI9225_GAMMA_CONTROL_10	0x59
> +
> +static int ili9225_buf_copy(void *dst, struct drm_framebuffer *fb,
> +			    struct drm_clip_rect *clip, bool swap)
> +{
> +	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> +	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
> +	struct drm_format_name_buf format_name;
> +	void *src = cma_obj->vaddr;
> +	int ret = 0;
> +
> +	if (import_attach) {
> +		ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
> +					       DMA_FROM_DEVICE);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	switch (fb->format->format) {
> +	case DRM_FORMAT_RGB565:
> +		if (swap)
> +			tinydrm_swab16(dst, src, fb, clip);
> +		else
> +			tinydrm_memcpy(dst, src, fb, clip);
> +		break;
> +	case DRM_FORMAT_XRGB8888:
> +		tinydrm_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
> +		break;
> +	default:
> +		dev_err_once(fb->dev->dev, "Format is not supported: %s\n",
> +			     drm_get_format_name(fb->format->format,
> +						 &format_name));
> +		return -EINVAL;
> +	}
> +
> +	if (import_attach)
> +		ret = dma_buf_end_cpu_access(import_attach->dmabuf,
> +					     DMA_FROM_DEVICE);
> +	return ret;
> +}
> +
> +static int ili9225_fb_dirty(struct drm_framebuffer *fb,
> +			    struct drm_file *file_priv, unsigned int flags,
> +			    unsigned int color, struct drm_clip_rect *clips,
> +			    unsigned int num_clips)
> +{
> +	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> +	struct tinydrm_device *tdev = fb->dev->dev_private;
> +	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> +	bool swap = mipi->swap_bytes;
> +	struct drm_clip_rect clip;
> +	u8 x_start, y_start;
> +	u8 x1, x2, y1, y2;
> +	int ret = 0;
> +	bool full;
> +	void *tr;
> +
> +	mutex_lock(&tdev->dirty_lock);
> +
> +	if (!mipi->enabled)
> +		goto out_unlock;
> +
> +	/* fbdev can flush even when we're not interested */
> +	if (tdev->pipe.plane.fb != fb)
> +		goto out_unlock;
> +
> +	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
> +				   fb->width, fb->height);
> +
> +	DRM_DEBUG("Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n", fb->base.id,
> +		  clip.x1, clip.x2, clip.y1, clip.y2);
> +
> +	if (!mipi->dc || !full || swap ||
> +	    fb->format->format == DRM_FORMAT_XRGB8888) {
> +		tr = mipi->tx_buf;
> +		ret = ili9225_buf_copy(mipi->tx_buf, fb, &clip, swap);
> +		if (ret)
> +			goto out_unlock;
> +	} else {
> +		tr = cma_obj->vaddr;
> +	}
> +
> +	switch (mipi->rotation) {
> +	default:
> +		x1 = clip.x1;
> +		x2 = clip.x2 - 1;
> +		y1 = clip.y1;
> +		y2 = clip.y2 - 1;
> +		x_start = x1;
> +		y_start = y1;
> +		break;
> +	case 90:
> +		x1 = clip.y1;
> +		x2 = clip.y2 - 1;
> +		y1 = fb->width - clip.x2;
> +		y2 = fb->width - clip.x1 - 1;
> +		x_start = x1;
> +		y_start = y2;
> +		break;
> +	case 180:
> +		x1 = fb->width - clip.x2;
> +		x2 = fb->width - clip.x1 - 1;
> +		y1 = fb->height - clip.y2;
> +		y2 = fb->height - clip.y1 - 1;
> +		x_start = x2;
> +		y_start = y2;
> +		break;
> +	case 270:
> +		x1 = fb->height - clip.y2;
> +		x2 = fb->height - clip.y1 - 1;
> +		y1 = clip.x1;
> +		y2 = clip.x2 - 1;
> +		x_start = x2;
> +		y_start = y1;
> +		break;
> +	}
> +
> +	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_1, 0x00, x2);

Please make a register write function that takes the reg value as 16-bit.
Like ili9225_write(mipi, u8 reg, u16 val) or something.

I have come to understand that mipi isn't really a good variable name here.
mipi can be a lot of things, MIPI DSI for instance uses dsi as a variable
name. So we really should use 'dbi' instead of 'mipi'.

Noralf.

> +	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_2, 0x00, x1);
> +	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_1, 0x00, y2);
> +	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_2, 0x00, y1);
> +
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, x_start);
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, y_start);
> +
> +	ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr,
> +				(clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
> +
> +out_unlock:
> +	mutex_unlock(&tdev->dirty_lock);
> +
> +	if (ret)
> +		dev_err_once(fb->dev->dev, "Failed to update display %d\n",
> +			     ret);
> +
> +	return ret;
> +}
> +
> +static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
> +	.destroy	= drm_gem_fb_destroy,
> +	.create_handle	= drm_gem_fb_create_handle,
> +	.dirty		= ili9225_fb_dirty,
> +};
> +
> +static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
> +				struct drm_crtc_state *crtc_state)
> +{
> +	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> +	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> +	struct drm_framebuffer *fb = pipe->plane.fb;
> +	struct device *dev = tdev->drm->dev;
> +	int ret;
> +	u8 am_id;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	mipi_dbi_hw_reset(mipi);
> +
> +	/*
> +	 * There don't seem to be two example init sequences that match, so
> +	 * using the one from the popular Arduino library for this display.
> +	 * https://github.com/Nkawu/TFT_22_ILI9225/blob/master/src/TFT_22_ILI9225.cpp
> +	 */
> +
> +	ret = mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x00, 0x00);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
> +		return;
> +	}
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x00, 0x00);
> +
> +	msleep(40);
> +
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x18);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x61, 0x21);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x6f);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x49, 0x5f);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x08, 0x00);
> +
> +	msleep(10);
> +
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x10, 0x3b);
> +
> +	msleep(50);
> +
> +	switch (mipi->rotation) {
> +	default:
> +		am_id = 0x30;
> +		break;
> +	case 90:
> +		am_id = 0x18;
> +		break;
> +	case 180:
> +		am_id = 0x00;
> +		break;
> +	case 270:
> +		am_id = 0x28;
> +		break;
> +	}
> +	mipi_dbi_command(mipi, ILI9225_DRIVER_OUTPUT_CONTROL, 0x01, 0x1c);
> +	mipi_dbi_command(mipi, ILI9225_LCD_AC_DRIVING_CONTROL, 0x01, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_ENTRY_MODE, 0x10, am_id);
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_BLANK_PERIOD_CONTROL_1, 0x08, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_FRAME_CYCLE_CONTROL, 0x11, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_INTERFACE_CONTROL, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_OSCILLATION_CONTROL, 0x0d, 0x01);
> +	mipi_dbi_command(mipi, ILI9225_VCI_RECYCLING, 0x00, 0x20);
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, 0x00);
> +
> +	mipi_dbi_command(mipi, ILI9225_GATE_SCAN_CONTROL, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_1, 0x00, 0xdb);
> +	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_2, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_3, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_1, 0x00, 0xdb);
> +	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_2, 0x00, 0x00);
> +
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_1, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_2, 0x08, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_3, 0x08, 0x0a);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_4, 0x00, 0x0a);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_5, 0x0a, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_6, 0x08, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_7, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_8, 0x0a, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_9, 0x07, 0x10);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_10, 0x07, 0x10);
> +
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x12);
> +
> +	msleep(50);
> +
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x10, 0x17);
> +
> +	mipi->enabled = true;
> +
> +	if (fb)
> +		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
> +}
> +
> +static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> +	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	if (!mipi->enabled)
> +		return;
> +
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
> +	msleep(50);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x07);
> +	msleep(50);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x0a, 0x02);
> +
> +	mipi->enabled = false;
> +}
> +
> +static u32 ili9225_spi_cmd_max_speed(struct spi_device *spi, size_t len)
> +{
> +	if (len > 64)
> +		return 0; /* use default */
> +
> +	return min_t(u32, 10000000, spi->max_speed_hz);
> +}
> +
> +static int ili9225_command(struct mipi_dbi *mipi, u8 cmd, u8 *par, size_t num)
> +{
> +	struct spi_device *spi = mipi->spi;
> +	unsigned int bpw = 8;
> +	u32 speed_hz;
> +	int ret;
> +
> +	gpiod_set_value_cansleep(mipi->dc, 0);
> +	speed_hz = ili9225_spi_cmd_max_speed(spi, 1);
> +	ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
> +	if (ret || !num)
> +		return ret;
> +
> +	if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
> +		bpw = 16;
> +
> +	gpiod_set_value_cansleep(mipi->dc, 1);
> +	speed_hz = ili9225_spi_cmd_max_speed(spi, num);
> +
> +	return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num);
> +}
> +
> +static int ili9225_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
> +			    struct gpio_desc *dc)
> +{
> +	size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
> +	struct device *dev = &spi->dev;
> +	int ret;
> +
> +	if (tx_size < 16) {
> +		DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Even though it's not the SPI device that does DMA (the master does),
> +	 * the dma mask is necessary for the dma_alloc_wc() in
> +	 * drm_gem_cma_create(). The dma_addr returned will be a physical
> +	 * address which might be different from the bus address, but this is
> +	 * not a problem since the address will not be used.
> +	 * The virtual address is used in the transfer and the SPI core
> +	 * re-maps it on the SPI master device using the DMA streaming API
> +	 * (spi_map_buf()).
> +	 */
> +	if (!dev->coherent_dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret) {
> +			dev_warn(dev, "Failed to set dma mask %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	mipi->spi = spi;
> +
> +	mipi->command = ili9225_command;
> +	mipi->dc = dc;
> +	if (tinydrm_machine_little_endian() &&
> +	    !tinydrm_spi_bpw_supported(spi, 16))
> +		mipi->swap_bytes = true;
> +
> +	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
> +
> +	return 0;
> +}
> +
> +static const u32 ili9225_formats[] = {
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static int ili9225_init(struct device *dev, struct mipi_dbi *mipi,
> +			const struct drm_simple_display_pipe_funcs *pipe_funcs,
> +			struct drm_driver *driver,
> +			const struct drm_display_mode *mode,
> +			unsigned int rotation)
> +{
> +	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
> +	struct tinydrm_device *tdev = &mipi->tinydrm;
> +	int ret;
> +
> +	if (!mipi->command)
> +		return -EINVAL;
> +
> +	mutex_init(&mipi->cmdlock);
> +
> +	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
> +	if (!mipi->tx_buf)
> +		return -ENOMEM;
> +
> +	ret = devm_tinydrm_init(dev, tdev, &ili9225_fb_funcs, driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> +					DRM_MODE_CONNECTOR_VIRTUAL,
> +					ili9225_formats,
> +					ARRAY_SIZE(ili9225_formats), mode,
> +					rotation);
> +	if (ret)
> +		return ret;
> +
> +	tdev->drm->mode_config.preferred_depth = 16;
> +	mipi->rotation = rotation;
> +
> +	drm_mode_config_reset(tdev->drm);
> +
> +	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
> +		      tdev->drm->mode_config.preferred_depth, rotation);
> +
> +	return 0;
> +}
> +
> +static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
> +	.enable		= ili9225_pipe_enable,
> +	.disable	= ili9225_pipe_disable,
> +	.update		= tinydrm_display_pipe_update,
> +	.prepare_fb	= tinydrm_display_pipe_prepare_fb,
> +};
> +
> +static const struct drm_display_mode ili9225_mode = {
> +	TINYDRM_MODE(176, 220, 35, 44),
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
> +
> +static struct drm_driver ili9225_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> +				  DRIVER_ATOMIC,
> +	.fops			= &ili9225_fops,
> +	TINYDRM_GEM_DRIVER_OPS,
> +	.lastclose		= tinydrm_lastclose,
> +	.name			= "ili9225",
> +	.desc			= "Ilitek ILI9225",
> +	.date			= "20171106",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static const struct of_device_id ili9225_of_match[] = {
> +	{ .compatible = "generic,2.2in-176x220-ili9225-tft" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ili9225_of_match);
> +
> +static const struct spi_device_id ili9225_id[] = {
> +	{ "2.2in-176x220-ili9225-tft", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, ili9225_id);
> +
> +static int ili9225_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct mipi_dbi *mipi;
> +	struct gpio_desc *rs;
> +	u32 rotation = 0;
> +	int ret;
> +
> +	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> +	if (!mipi)
> +		return -ENOMEM;
> +
> +	mipi->reset = devm_gpiod_get(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);
> +	}
> +
> +	rs = devm_gpiod_get(dev, "rs", GPIOD_OUT_LOW);
> +	if (IS_ERR(rs)) {
> +		DRM_DEV_ERROR(dev, "Failed to get gpio 'rs'\n");
> +		return PTR_ERR(rs);
> +	}
> +
> +	device_property_read_u32(dev, "rotation", &rotation);
> +
> +	ret = ili9225_spi_init(spi, mipi, rs);
> +	if (ret)
> +		return ret;
> +
> +	ret = ili9225_init(&spi->dev, mipi, &ili9225_pipe_funcs,
> +			   &ili9225_driver, &ili9225_mode, rotation);
> +	if (ret)
> +		return ret;
> +
> +	spi_set_drvdata(spi, mipi);
> +
> +	return devm_tinydrm_register(&mipi->tinydrm);
> +}
> +
> +static void ili9225_shutdown(struct spi_device *spi)
> +{
> +	struct mipi_dbi *mipi = spi_get_drvdata(spi);
> +
> +	tinydrm_shutdown(&mipi->tinydrm);
> +}
> +
> +static struct spi_driver ili9225_spi_driver = {
> +	.driver = {
> +		.name = "ili9225",
> +		.owner = THIS_MODULE,
> +		.of_match_table = ili9225_of_match,
> +	},
> +	.id_table = ili9225_id,
> +	.probe = ili9225_probe,
> +	.shutdown = ili9225_shutdown,
> +};
> +module_spi_driver(ili9225_spi_driver);
> +
> +MODULE_DESCRIPTION("Ilitek ILI9225 DRM driver");
> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
> +MODULE_LICENSE("GPL");

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-11-10 18:44     ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2017-11-10 18:44 UTC (permalink / raw)
  To: David Lechner, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Rob Herring, Mark Rutland, linux-kernel-u79uwXL29TY76Z2rM5mHXA


Den 08.11.2017 04.52, skrev David Lechner:
> This adds a new driver for display panels based on the Ilitek ILI9225
> controller.
>
> This was developed for a no-name panel with a red PCB that is commonly
> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>
> I really did try very hard to find a make and model for this panel, but
> there doesn't seem to be one, so the best I can do is offer the picture
> in the link above for identification.
>
> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> ---
>   MAINTAINERS                       |   6 +
>   drivers/gpu/drm/tinydrm/Kconfig   |  10 +
>   drivers/gpu/drm/tinydrm/Makefile  |   1 +
>   drivers/gpu/drm/tinydrm/ili9225.c | 547 ++++++++++++++++++++++++++++++++++++++
>   4 files changed, 564 insertions(+)
>   create mode 100644 drivers/gpu/drm/tinydrm/ili9225.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0d77f22..72404f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4372,6 +4372,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
>   S:	Maintained
>   F:	drivers/gpu/drm/tve200/
>   
> +DRM DRIVER FOR ILITEK ILI9225 PANELS
> +M:	David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> +S:	Maintained
> +F:	drivers/gpu/drm/tinydrm/ili9225.c
> +F:	Documentation/devicetree/bindings/display/ili9225.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 2e790e7..90c5bd5 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -12,6 +12,16 @@ menuconfig DRM_TINYDRM
>   config TINYDRM_MIPI_DBI
>   	tristate
>   
> +config TINYDRM_ILI9225
> +	tristate "DRM support for ILI9225 display panels"
> +	depends on DRM_TINYDRM && SPI
> +	select TINYDRM_MIPI_DBI
> +	help
> +	  DRM driver for the following Ilitek ILI9225 panels:
> +	  * No-name 2.2" color screen module
> +
> +	  If M is selected the module will be called ili9225.
> +
>   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 0c184bd..8aeee53 100644
> --- a/drivers/gpu/drm/tinydrm/Makefile
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
>   obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
>   
>   # Displays
> +obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.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/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
> new file mode 100644
> index 0000000..07e1b8b
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
> @@ -0,0 +1,547 @@
> +/*
> + * DRM driver for Ilitek ILI9225 panels
> + *
> + * Copyright 2017 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> + *
> + * Lots of code copied from mipi-dbi.c
> + * Copyright 2016 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/dma-buf.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/spi/spi.h>
> +#include <video/mipi_display.h>
> +
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/tinydrm/mipi-dbi.h>
> +#include <drm/tinydrm/tinydrm-helpers.h>
> +
> +#define ILI9225_DRIVER_READ_CODE	0x00
> +#define ILI9225_DRIVER_OUTPUT_CONTROL	0x01
> +#define ILI9225_LCD_AC_DRIVING_CONTROL	0x02
> +#define ILI9225_ENTRY_MODE		0x03
> +#define ILI9225_DISPLAY_CONTROL_1	0x07
> +#define ILI9225_BLANK_PERIOD_CONTROL_1	0x08
> +#define ILI9225_FRAME_CYCLE_CONTROL	0x0b
> +#define ILI9225_INTERFACE_CONTROL	0x0c
> +#define ILI9225_OSCILLATION_CONTROL	0x0f
> +#define ILI9225_POWER_CONTROL_1		0x10
> +#define ILI9225_POWER_CONTROL_2		0x11
> +#define ILI9225_POWER_CONTROL_3		0x12
> +#define ILI9225_POWER_CONTROL_4		0x13
> +#define ILI9225_POWER_CONTROL_5		0x14
> +#define ILI9225_VCI_RECYCLING		0x15
> +#define ILI9225_RAM_ADDRESS_SET_1	0x20
> +#define ILI9225_RAM_ADDRESS_SET_2	0x21
> +#define ILI9225_WRITE_DATA_TO_GRAM	0x22
> +#define ILI9225_SOFTWARE_RESET		0x28
> +#define ILI9225_GATE_SCAN_CONTROL	0x30
> +#define ILI9225_VERTICAL_SCROLL_1	0x31
> +#define ILI9225_VERTICAL_SCROLL_2	0x32
> +#define ILI9225_VERTICAL_SCROLL_3	0x33
> +#define ILI9225_PARTIAL_DRIVING_POS_1	0x34
> +#define ILI9225_PARTIAL_DRIVING_POS_2	0x35
> +#define ILI9225_HORIZ_WINDOW_ADDR_1	0x36
> +#define ILI9225_HORIZ_WINDOW_ADDR_2	0x37
> +#define ILI9225_VERT_WINDOW_ADDR_1	0x38
> +#define ILI9225_VERT_WINDOW_ADDR_2	0x39
> +#define ILI9225_GAMMA_CONTROL_1		0x50
> +#define ILI9225_GAMMA_CONTROL_2		0x51
> +#define ILI9225_GAMMA_CONTROL_3		0x52
> +#define ILI9225_GAMMA_CONTROL_4		0x53
> +#define ILI9225_GAMMA_CONTROL_5		0x54
> +#define ILI9225_GAMMA_CONTROL_6		0x55
> +#define ILI9225_GAMMA_CONTROL_7		0x56
> +#define ILI9225_GAMMA_CONTROL_8		0x57
> +#define ILI9225_GAMMA_CONTROL_9		0x58
> +#define ILI9225_GAMMA_CONTROL_10	0x59
> +
> +static int ili9225_buf_copy(void *dst, struct drm_framebuffer *fb,
> +			    struct drm_clip_rect *clip, bool swap)
> +{
> +	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> +	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
> +	struct drm_format_name_buf format_name;
> +	void *src = cma_obj->vaddr;
> +	int ret = 0;
> +
> +	if (import_attach) {
> +		ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
> +					       DMA_FROM_DEVICE);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	switch (fb->format->format) {
> +	case DRM_FORMAT_RGB565:
> +		if (swap)
> +			tinydrm_swab16(dst, src, fb, clip);
> +		else
> +			tinydrm_memcpy(dst, src, fb, clip);
> +		break;
> +	case DRM_FORMAT_XRGB8888:
> +		tinydrm_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
> +		break;
> +	default:
> +		dev_err_once(fb->dev->dev, "Format is not supported: %s\n",
> +			     drm_get_format_name(fb->format->format,
> +						 &format_name));
> +		return -EINVAL;
> +	}
> +
> +	if (import_attach)
> +		ret = dma_buf_end_cpu_access(import_attach->dmabuf,
> +					     DMA_FROM_DEVICE);
> +	return ret;
> +}
> +
> +static int ili9225_fb_dirty(struct drm_framebuffer *fb,
> +			    struct drm_file *file_priv, unsigned int flags,
> +			    unsigned int color, struct drm_clip_rect *clips,
> +			    unsigned int num_clips)
> +{
> +	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> +	struct tinydrm_device *tdev = fb->dev->dev_private;
> +	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> +	bool swap = mipi->swap_bytes;
> +	struct drm_clip_rect clip;
> +	u8 x_start, y_start;
> +	u8 x1, x2, y1, y2;
> +	int ret = 0;
> +	bool full;
> +	void *tr;
> +
> +	mutex_lock(&tdev->dirty_lock);
> +
> +	if (!mipi->enabled)
> +		goto out_unlock;
> +
> +	/* fbdev can flush even when we're not interested */
> +	if (tdev->pipe.plane.fb != fb)
> +		goto out_unlock;
> +
> +	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
> +				   fb->width, fb->height);
> +
> +	DRM_DEBUG("Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n", fb->base.id,
> +		  clip.x1, clip.x2, clip.y1, clip.y2);
> +
> +	if (!mipi->dc || !full || swap ||
> +	    fb->format->format == DRM_FORMAT_XRGB8888) {
> +		tr = mipi->tx_buf;
> +		ret = ili9225_buf_copy(mipi->tx_buf, fb, &clip, swap);
> +		if (ret)
> +			goto out_unlock;
> +	} else {
> +		tr = cma_obj->vaddr;
> +	}
> +
> +	switch (mipi->rotation) {
> +	default:
> +		x1 = clip.x1;
> +		x2 = clip.x2 - 1;
> +		y1 = clip.y1;
> +		y2 = clip.y2 - 1;
> +		x_start = x1;
> +		y_start = y1;
> +		break;
> +	case 90:
> +		x1 = clip.y1;
> +		x2 = clip.y2 - 1;
> +		y1 = fb->width - clip.x2;
> +		y2 = fb->width - clip.x1 - 1;
> +		x_start = x1;
> +		y_start = y2;
> +		break;
> +	case 180:
> +		x1 = fb->width - clip.x2;
> +		x2 = fb->width - clip.x1 - 1;
> +		y1 = fb->height - clip.y2;
> +		y2 = fb->height - clip.y1 - 1;
> +		x_start = x2;
> +		y_start = y2;
> +		break;
> +	case 270:
> +		x1 = fb->height - clip.y2;
> +		x2 = fb->height - clip.y1 - 1;
> +		y1 = clip.x1;
> +		y2 = clip.x2 - 1;
> +		x_start = x2;
> +		y_start = y1;
> +		break;
> +	}
> +
> +	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_1, 0x00, x2);

Please make a register write function that takes the reg value as 16-bit.
Like ili9225_write(mipi, u8 reg, u16 val) or something.

I have come to understand that mipi isn't really a good variable name here.
mipi can be a lot of things, MIPI DSI for instance uses dsi as a variable
name. So we really should use 'dbi' instead of 'mipi'.

Noralf.

> +	mipi_dbi_command(mipi, ILI9225_HORIZ_WINDOW_ADDR_2, 0x00, x1);
> +	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_1, 0x00, y2);
> +	mipi_dbi_command(mipi, ILI9225_VERT_WINDOW_ADDR_2, 0x00, y1);
> +
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, x_start);
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, y_start);
> +
> +	ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr,
> +				(clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
> +
> +out_unlock:
> +	mutex_unlock(&tdev->dirty_lock);
> +
> +	if (ret)
> +		dev_err_once(fb->dev->dev, "Failed to update display %d\n",
> +			     ret);
> +
> +	return ret;
> +}
> +
> +static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
> +	.destroy	= drm_gem_fb_destroy,
> +	.create_handle	= drm_gem_fb_create_handle,
> +	.dirty		= ili9225_fb_dirty,
> +};
> +
> +static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
> +				struct drm_crtc_state *crtc_state)
> +{
> +	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> +	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> +	struct drm_framebuffer *fb = pipe->plane.fb;
> +	struct device *dev = tdev->drm->dev;
> +	int ret;
> +	u8 am_id;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	mipi_dbi_hw_reset(mipi);
> +
> +	/*
> +	 * There don't seem to be two example init sequences that match, so
> +	 * using the one from the popular Arduino library for this display.
> +	 * https://github.com/Nkawu/TFT_22_ILI9225/blob/master/src/TFT_22_ILI9225.cpp
> +	 */
> +
> +	ret = mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x00, 0x00);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
> +		return;
> +	}
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x00, 0x00);
> +
> +	msleep(40);
> +
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x18);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_3, 0x61, 0x21);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_4, 0x00, 0x6f);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_5, 0x49, 0x5f);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x08, 0x00);
> +
> +	msleep(10);
> +
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x10, 0x3b);
> +
> +	msleep(50);
> +
> +	switch (mipi->rotation) {
> +	default:
> +		am_id = 0x30;
> +		break;
> +	case 90:
> +		am_id = 0x18;
> +		break;
> +	case 180:
> +		am_id = 0x00;
> +		break;
> +	case 270:
> +		am_id = 0x28;
> +		break;
> +	}
> +	mipi_dbi_command(mipi, ILI9225_DRIVER_OUTPUT_CONTROL, 0x01, 0x1c);
> +	mipi_dbi_command(mipi, ILI9225_LCD_AC_DRIVING_CONTROL, 0x01, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_ENTRY_MODE, 0x10, am_id);
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_BLANK_PERIOD_CONTROL_1, 0x08, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_FRAME_CYCLE_CONTROL, 0x11, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_INTERFACE_CONTROL, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_OSCILLATION_CONTROL, 0x0d, 0x01);
> +	mipi_dbi_command(mipi, ILI9225_VCI_RECYCLING, 0x00, 0x20);
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_1, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_RAM_ADDRESS_SET_2, 0x00, 0x00);
> +
> +	mipi_dbi_command(mipi, ILI9225_GATE_SCAN_CONTROL, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_1, 0x00, 0xdb);
> +	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_2, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_VERTICAL_SCROLL_3, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_1, 0x00, 0xdb);
> +	mipi_dbi_command(mipi, ILI9225_PARTIAL_DRIVING_POS_2, 0x00, 0x00);
> +
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_1, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_2, 0x08, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_3, 0x08, 0x0a);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_4, 0x00, 0x0a);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_5, 0x0a, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_6, 0x08, 0x08);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_7, 0x00, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_8, 0x0a, 0x00);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_9, 0x07, 0x10);
> +	mipi_dbi_command(mipi, ILI9225_GAMMA_CONTROL_10, 0x07, 0x10);
> +
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x12);
> +
> +	msleep(50);
> +
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x10, 0x17);
> +
> +	mipi->enabled = true;
> +
> +	if (fb)
> +		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
> +}
> +
> +static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> +	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	if (!mipi->enabled)
> +		return;
> +
> +	mipi_dbi_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x00, 0x00);
> +	msleep(50);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_2, 0x00, 0x07);
> +	msleep(50);
> +	mipi_dbi_command(mipi, ILI9225_POWER_CONTROL_1, 0x0a, 0x02);
> +
> +	mipi->enabled = false;
> +}
> +
> +static u32 ili9225_spi_cmd_max_speed(struct spi_device *spi, size_t len)
> +{
> +	if (len > 64)
> +		return 0; /* use default */
> +
> +	return min_t(u32, 10000000, spi->max_speed_hz);
> +}
> +
> +static int ili9225_command(struct mipi_dbi *mipi, u8 cmd, u8 *par, size_t num)
> +{
> +	struct spi_device *spi = mipi->spi;
> +	unsigned int bpw = 8;
> +	u32 speed_hz;
> +	int ret;
> +
> +	gpiod_set_value_cansleep(mipi->dc, 0);
> +	speed_hz = ili9225_spi_cmd_max_speed(spi, 1);
> +	ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
> +	if (ret || !num)
> +		return ret;
> +
> +	if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
> +		bpw = 16;
> +
> +	gpiod_set_value_cansleep(mipi->dc, 1);
> +	speed_hz = ili9225_spi_cmd_max_speed(spi, num);
> +
> +	return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num);
> +}
> +
> +static int ili9225_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
> +			    struct gpio_desc *dc)
> +{
> +	size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
> +	struct device *dev = &spi->dev;
> +	int ret;
> +
> +	if (tx_size < 16) {
> +		DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Even though it's not the SPI device that does DMA (the master does),
> +	 * the dma mask is necessary for the dma_alloc_wc() in
> +	 * drm_gem_cma_create(). The dma_addr returned will be a physical
> +	 * address which might be different from the bus address, but this is
> +	 * not a problem since the address will not be used.
> +	 * The virtual address is used in the transfer and the SPI core
> +	 * re-maps it on the SPI master device using the DMA streaming API
> +	 * (spi_map_buf()).
> +	 */
> +	if (!dev->coherent_dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret) {
> +			dev_warn(dev, "Failed to set dma mask %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	mipi->spi = spi;
> +
> +	mipi->command = ili9225_command;
> +	mipi->dc = dc;
> +	if (tinydrm_machine_little_endian() &&
> +	    !tinydrm_spi_bpw_supported(spi, 16))
> +		mipi->swap_bytes = true;
> +
> +	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
> +
> +	return 0;
> +}
> +
> +static const u32 ili9225_formats[] = {
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static int ili9225_init(struct device *dev, struct mipi_dbi *mipi,
> +			const struct drm_simple_display_pipe_funcs *pipe_funcs,
> +			struct drm_driver *driver,
> +			const struct drm_display_mode *mode,
> +			unsigned int rotation)
> +{
> +	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
> +	struct tinydrm_device *tdev = &mipi->tinydrm;
> +	int ret;
> +
> +	if (!mipi->command)
> +		return -EINVAL;
> +
> +	mutex_init(&mipi->cmdlock);
> +
> +	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
> +	if (!mipi->tx_buf)
> +		return -ENOMEM;
> +
> +	ret = devm_tinydrm_init(dev, tdev, &ili9225_fb_funcs, driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> +					DRM_MODE_CONNECTOR_VIRTUAL,
> +					ili9225_formats,
> +					ARRAY_SIZE(ili9225_formats), mode,
> +					rotation);
> +	if (ret)
> +		return ret;
> +
> +	tdev->drm->mode_config.preferred_depth = 16;
> +	mipi->rotation = rotation;
> +
> +	drm_mode_config_reset(tdev->drm);
> +
> +	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
> +		      tdev->drm->mode_config.preferred_depth, rotation);
> +
> +	return 0;
> +}
> +
> +static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
> +	.enable		= ili9225_pipe_enable,
> +	.disable	= ili9225_pipe_disable,
> +	.update		= tinydrm_display_pipe_update,
> +	.prepare_fb	= tinydrm_display_pipe_prepare_fb,
> +};
> +
> +static const struct drm_display_mode ili9225_mode = {
> +	TINYDRM_MODE(176, 220, 35, 44),
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
> +
> +static struct drm_driver ili9225_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> +				  DRIVER_ATOMIC,
> +	.fops			= &ili9225_fops,
> +	TINYDRM_GEM_DRIVER_OPS,
> +	.lastclose		= tinydrm_lastclose,
> +	.name			= "ili9225",
> +	.desc			= "Ilitek ILI9225",
> +	.date			= "20171106",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static const struct of_device_id ili9225_of_match[] = {
> +	{ .compatible = "generic,2.2in-176x220-ili9225-tft" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ili9225_of_match);
> +
> +static const struct spi_device_id ili9225_id[] = {
> +	{ "2.2in-176x220-ili9225-tft", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, ili9225_id);
> +
> +static int ili9225_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct mipi_dbi *mipi;
> +	struct gpio_desc *rs;
> +	u32 rotation = 0;
> +	int ret;
> +
> +	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> +	if (!mipi)
> +		return -ENOMEM;
> +
> +	mipi->reset = devm_gpiod_get(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);
> +	}
> +
> +	rs = devm_gpiod_get(dev, "rs", GPIOD_OUT_LOW);
> +	if (IS_ERR(rs)) {
> +		DRM_DEV_ERROR(dev, "Failed to get gpio 'rs'\n");
> +		return PTR_ERR(rs);
> +	}
> +
> +	device_property_read_u32(dev, "rotation", &rotation);
> +
> +	ret = ili9225_spi_init(spi, mipi, rs);
> +	if (ret)
> +		return ret;
> +
> +	ret = ili9225_init(&spi->dev, mipi, &ili9225_pipe_funcs,
> +			   &ili9225_driver, &ili9225_mode, rotation);
> +	if (ret)
> +		return ret;
> +
> +	spi_set_drvdata(spi, mipi);
> +
> +	return devm_tinydrm_register(&mipi->tinydrm);
> +}
> +
> +static void ili9225_shutdown(struct spi_device *spi)
> +{
> +	struct mipi_dbi *mipi = spi_get_drvdata(spi);
> +
> +	tinydrm_shutdown(&mipi->tinydrm);
> +}
> +
> +static struct spi_driver ili9225_spi_driver = {
> +	.driver = {
> +		.name = "ili9225",
> +		.owner = THIS_MODULE,
> +		.of_match_table = ili9225_of_match,
> +	},
> +	.id_table = ili9225_id,
> +	.probe = ili9225_probe,
> +	.shutdown = ili9225_shutdown,
> +};
> +module_spi_driver(ili9225_spi_driver);
> +
> +MODULE_DESCRIPTION("Ilitek ILI9225 DRM driver");
> +MODULE_AUTHOR("David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>");
> +MODULE_LICENSE("GPL");

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v1 1/2] dt-bindings: Add binding for Ilitek ILI9225 display panels
@ 2017-11-10 21:31     ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2017-11-10 21:31 UTC (permalink / raw)
  To: David Lechner; +Cc: dri-devel, devicetree, Mark Rutland, linux-kernel

On Tue, Nov 07, 2017 at 09:52:15PM -0600, David Lechner wrote:
> This adds a new binding for display panels that use an Ilitek ILI9225
> controller.
> 
> The "generic,2.2in-176x220-ili9225-tft" device listed has no identification
> markings whatsoever and an hour of googling turned up nothing, hence the use
> of "generic".

Just use ilitek then. I would do "ilitek,ili9225-2.2in-176x220" and 
hopefully there aren't others that size. I'm guessing you would have 
found them if so.

Otherwise, looks fine.

> An example of this nameless device can be found at:
> https://github.com/Nkawu/TFT_22_ILI9225
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  .../devicetree/bindings/display/ilitek,ili9225.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt

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

* Re: [PATCH v1 1/2] dt-bindings: Add binding for Ilitek ILI9225 display panels
@ 2017-11-10 21:31     ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2017-11-10 21:31 UTC (permalink / raw)
  To: David Lechner
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tue, Nov 07, 2017 at 09:52:15PM -0600, David Lechner wrote:
> This adds a new binding for display panels that use an Ilitek ILI9225
> controller.
> 
> The "generic,2.2in-176x220-ili9225-tft" device listed has no identification
> markings whatsoever and an hour of googling turned up nothing, hence the use
> of "generic".

Just use ilitek then. I would do "ilitek,ili9225-2.2in-176x220" and 
hopefully there aren't others that size. I'm guessing you would have 
found them if so.

Otherwise, looks fine.

> An example of this nameless device can be found at:
> https://github.com/Nkawu/TFT_22_ILI9225
> 
> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> ---
>  .../devicetree/bindings/display/ilitek,ili9225.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9225.txt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
  2017-11-08  3:52   ` David Lechner
@ 2017-12-01 14:03     ` Linus Walleij
  -1 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2017-12-01 14:03 UTC (permalink / raw)
  To: David Lechner, Stefano Babic
  Cc: open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Mark Rutland, Rob Herring, linux-kernel

On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <david@lechnology.com> wrote:

> This adds a new driver for display panels based on the Ilitek ILI9225
> controller.
>
> This was developed for a no-name panel with a red PCB that is commonly
> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>
> I really did try very hard to find a make and model for this panel, but
> there doesn't seem to be one, so the best I can do is offer the picture
> in the link above for identification.
>
> Signed-off-by: David Lechner <david@lechnology.com>

Can you explain why tinydrm is not putting its panel drivers in
drivers/gpu/drm/panel?

I guess everybody knows except me, it's usually like that :(

I am anyways working on a driver for Ilitek 9322 that I want
to land in drivers/gpu/drm/panel. Here is the last iteration:
https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
Yeah I got sidetracked. OK I will get to it now.

There are some similarities with the code I'm seeing here
but I believe they are essentially different. But it will be hard
to share code if you put the driver in the tinydrm framework.

I guess you have also seen:
drivers/video/backlight/ili922x.c
?

Stefano Babic who wrote the backlight driver is available for
reviewing, so includ him in follow-ups (added to To: line).

I'm putting you on CC as I'm rewriting it a bit after the DT
maintainers review, will try to repost ASAP.

Yours,
Linus Walleij

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-12-01 14:03     ` Linus Walleij
  0 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2017-12-01 14:03 UTC (permalink / raw)
  To: David Lechner, Stefano Babic
  Cc: open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Mark Rutland, Rob Herring, linux-kernel

On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <david@lechnology.com> wrote:

> This adds a new driver for display panels based on the Ilitek ILI9225
> controller.
>
> This was developed for a no-name panel with a red PCB that is commonly
> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>
> I really did try very hard to find a make and model for this panel, but
> there doesn't seem to be one, so the best I can do is offer the picture
> in the link above for identification.
>
> Signed-off-by: David Lechner <david@lechnology.com>

Can you explain why tinydrm is not putting its panel drivers in
drivers/gpu/drm/panel?

I guess everybody knows except me, it's usually like that :(

I am anyways working on a driver for Ilitek 9322 that I want
to land in drivers/gpu/drm/panel. Here is the last iteration:
https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
Yeah I got sidetracked. OK I will get to it now.

There are some similarities with the code I'm seeing here
but I believe they are essentially different. But it will be hard
to share code if you put the driver in the tinydrm framework.

I guess you have also seen:
drivers/video/backlight/ili922x.c
?

Stefano Babic who wrote the backlight driver is available for
reviewing, so includ him in follow-ups (added to To: line).

I'm putting you on CC as I'm rewriting it a bit after the DT
maintainers review, will try to repost ASAP.

Yours,
Linus Walleij

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-12-01 18:27       ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2017-12-01 18:27 UTC (permalink / raw)
  To: Linus Walleij, David Lechner, Stefano Babic
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring, linux-kernel, open list:DRM PANEL DRIVERS,
	Thierry Reding

(cc: Thierry)

Den 01.12.2017 15.03, skrev Linus Walleij:
> On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <david@lechnology.com> wrote:
>
>> This adds a new driver for display panels based on the Ilitek ILI9225
>> controller.
>>
>> This was developed for a no-name panel with a red PCB that is commonly
>> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>>
>> I really did try very hard to find a make and model for this panel, but
>> there doesn't seem to be one, so the best I can do is offer the picture
>> in the link above for identification.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
> Can you explain why tinydrm is not putting its panel drivers in
> drivers/gpu/drm/panel?

The short answer is that tinydrm sends pixel data over the panel
controller's control interface whereas drm/panel leaves that to a
dedicated pixel interface fed by another driver.

tinydrm is used with controllers that have onboard GRAM which is
scanned out by the panel controller itself. Many of these controllers
support multiple interfaces, like MIPI DSI/DPI/DBI. A MIPI DPI
compatible controller has a control bus, usually SPI, to operate the
panel. This same control bus can be used in MIPI DBI mode on some
controllers to push pixels.

The MIPI standard documents isn't open, but some are available if you
do a search. We also have MIPI DCS which is the command set shared by
the MIPI compatible controllers.

So how do we deal with controllers that can operate in many modes?
I raised the question when a panel driver was reviewed earlier this
year, but nothing really came out of it.

I suppose that displays that are used with DSI/DPI doesn't have a
controller with onboard GRAM, since that would just increase the price.
The MIPI standard defines different controller types which has no,
partial or full framebuffer. So in reality it's not that likely that
we will see the same controller used both in tinydrm and drm/panel.

But I'm hardly an expert on these matters, I've only used the DBI mode.
I've cc'ed the drm/panel maintainer, maybe he can shed some more light.

Noralf.

> I guess everybody knows except me, it's usually like that :(
>
> I am anyways working on a driver for Ilitek 9322 that I want
> to land in drivers/gpu/drm/panel. Here is the last iteration:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
> Yeah I got sidetracked. OK I will get to it now.
>
> There are some similarities with the code I'm seeing here
> but I believe they are essentially different. But it will be hard
> to share code if you put the driver in the tinydrm framework.
>
> I guess you have also seen:
> drivers/video/backlight/ili922x.c
> ?
>
> Stefano Babic who wrote the backlight driver is available for
> reviewing, so includ him in follow-ups (added to To: line).
>
> I'm putting you on CC as I'm rewriting it a bit after the DT
> maintainers review, will try to repost ASAP.
>
> Yours,
> Linus Walleij
> _______________________________________________
> 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 v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-12-01 18:27       ` Noralf Trønnes
  0 siblings, 0 replies; 18+ messages in thread
From: Noralf Trønnes @ 2017-12-01 18:27 UTC (permalink / raw)
  To: Linus Walleij, David Lechner, Stefano Babic
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	open list:DRM PANEL DRIVERS, Thierry Reding

(cc: Thierry)

Den 01.12.2017 15.03, skrev Linus Walleij:
> On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> wrote:
>
>> This adds a new driver for display panels based on the Ilitek ILI9225
>> controller.
>>
>> This was developed for a no-name panel with a red PCB that is commonly
>> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>>
>> I really did try very hard to find a make and model for this panel, but
>> there doesn't seem to be one, so the best I can do is offer the picture
>> in the link above for identification.
>>
>> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> Can you explain why tinydrm is not putting its panel drivers in
> drivers/gpu/drm/panel?

The short answer is that tinydrm sends pixel data over the panel
controller's control interface whereas drm/panel leaves that to a
dedicated pixel interface fed by another driver.

tinydrm is used with controllers that have onboard GRAM which is
scanned out by the panel controller itself. Many of these controllers
support multiple interfaces, like MIPI DSI/DPI/DBI. A MIPI DPI
compatible controller has a control bus, usually SPI, to operate the
panel. This same control bus can be used in MIPI DBI mode on some
controllers to push pixels.

The MIPI standard documents isn't open, but some are available if you
do a search. We also have MIPI DCS which is the command set shared by
the MIPI compatible controllers.

So how do we deal with controllers that can operate in many modes?
I raised the question when a panel driver was reviewed earlier this
year, but nothing really came out of it.

I suppose that displays that are used with DSI/DPI doesn't have a
controller with onboard GRAM, since that would just increase the price.
The MIPI standard defines different controller types which has no,
partial or full framebuffer. So in reality it's not that likely that
we will see the same controller used both in tinydrm and drm/panel.

But I'm hardly an expert on these matters, I've only used the DBI mode.
I've cc'ed the drm/panel maintainer, maybe he can shed some more light.

Noralf.

> I guess everybody knows except me, it's usually like that :(
>
> I am anyways working on a driver for Ilitek 9322 that I want
> to land in drivers/gpu/drm/panel. Here is the last iteration:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
> Yeah I got sidetracked. OK I will get to it now.
>
> There are some similarities with the code I'm seeing here
> but I believe they are essentially different. But it will be hard
> to share code if you put the driver in the tinydrm framework.
>
> I guess you have also seen:
> drivers/video/backlight/ili922x.c
> ?
>
> Stefano Babic who wrote the backlight driver is available for
> reviewing, so includ him in follow-ups (added to To: line).
>
> I'm putting you on CC as I'm rewriting it a bit after the DT
> maintainers review, will try to repost ASAP.
>
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
  2017-12-01 14:03     ` Linus Walleij
@ 2017-12-04  7:45       ` Daniel Vetter
  -1 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2017-12-04  7:45 UTC (permalink / raw)
  To: Linus Walleij
  Cc: David Lechner, Stefano Babic, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring, linux-kernel, open list:DRM PANEL DRIVERS

On Fri, Dec 01, 2017 at 03:03:30PM +0100, Linus Walleij wrote:
> On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <david@lechnology.com> wrote:
> 
> > This adds a new driver for display panels based on the Ilitek ILI9225
> > controller.
> >
> > This was developed for a no-name panel with a red PCB that is commonly
> > marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
> >
> > I really did try very hard to find a make and model for this panel, but
> > there doesn't seem to be one, so the best I can do is offer the picture
> > in the link above for identification.
> >
> > Signed-off-by: David Lechner <david@lechnology.com>
> 
> Can you explain why tinydrm is not putting its panel drivers in
> drivers/gpu/drm/panel?
> 
> I guess everybody knows except me, it's usually like that :(
> 
> I am anyways working on a driver for Ilitek 9322 that I want
> to land in drivers/gpu/drm/panel. Here is the last iteration:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
> Yeah I got sidetracked. OK I will get to it now.
> 
> There are some similarities with the code I'm seeing here
> but I believe they are essentially different. But it will be hard
> to share code if you put the driver in the tinydrm framework.
> 
> I guess you have also seen:
> drivers/video/backlight/ili922x.c
> ?
> 
> Stefano Babic who wrote the backlight driver is available for
> reviewing, so includ him in follow-ups (added to To: line).
> 
> I'm putting you on CC as I'm rewriting it a bit after the DT
> maintainers review, will try to repost ASAP.

Bit more historical context: We tried using drm_panel in tinydrm, but that
didn't really fit to well (as Noralf explains, tinydrm is kinda more for
stand-alone panels). But tinydrm is also a bit too much midlayer-y still,
so there's a bunch of todo items capture in Documentation/gpu/todo.rst. In
the end we shouldn't need a special tinydrm driver, that should be covered
by the usual drm helpers.

Might be worth it to at least capture/summarize some of the reasons for
why tinydrm doesn't use drm_panel, and what it would take to better share
code (or maybe that's just a silly idea, not the first duplicated driver
in drm).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels
@ 2017-12-04  7:45       ` Daniel Vetter
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2017-12-04  7:45 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Lechner, linux-kernel, open list:DRM PANEL DRIVERS,
	Rob Herring, Stefano Babic

On Fri, Dec 01, 2017 at 03:03:30PM +0100, Linus Walleij wrote:
> On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <david@lechnology.com> wrote:
> 
> > This adds a new driver for display panels based on the Ilitek ILI9225
> > controller.
> >
> > This was developed for a no-name panel with a red PCB that is commonly
> > marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
> >
> > I really did try very hard to find a make and model for this panel, but
> > there doesn't seem to be one, so the best I can do is offer the picture
> > in the link above for identification.
> >
> > Signed-off-by: David Lechner <david@lechnology.com>
> 
> Can you explain why tinydrm is not putting its panel drivers in
> drivers/gpu/drm/panel?
> 
> I guess everybody knows except me, it's usually like that :(
> 
> I am anyways working on a driver for Ilitek 9322 that I want
> to land in drivers/gpu/drm/panel. Here is the last iteration:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
> Yeah I got sidetracked. OK I will get to it now.
> 
> There are some similarities with the code I'm seeing here
> but I believe they are essentially different. But it will be hard
> to share code if you put the driver in the tinydrm framework.
> 
> I guess you have also seen:
> drivers/video/backlight/ili922x.c
> ?
> 
> Stefano Babic who wrote the backlight driver is available for
> reviewing, so includ him in follow-ups (added to To: line).
> 
> I'm putting you on CC as I'm rewriting it a bit after the DT
> maintainers review, will try to repost ASAP.

Bit more historical context: We tried using drm_panel in tinydrm, but that
didn't really fit to well (as Noralf explains, tinydrm is kinda more for
stand-alone panels). But tinydrm is also a bit too much midlayer-y still,
so there's a bunch of todo items capture in Documentation/gpu/todo.rst. In
the end we shouldn't need a special tinydrm driver, that should be covered
by the usual drm helpers.

Might be worth it to at least capture/summarize some of the reasons for
why tinydrm doesn't use drm_panel, and what it would take to better share
code (or maybe that's just a silly idea, not the first duplicated driver
in drm).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
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:[~2017-12-04  7:45 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08  3:52 [PATCH v1 0/2] DRM driver for ILI9225 display panels David Lechner
2017-11-08  3:52 ` David Lechner
2017-11-08  3:52 ` [PATCH v1 1/2] dt-bindings: Add binding for Ilitek " David Lechner
2017-11-08  3:52   ` David Lechner
2017-11-10 21:31   ` Rob Herring
2017-11-10 21:31     ` Rob Herring
2017-11-08  3:52 ` [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels David Lechner
2017-11-08  3:52   ` David Lechner
2017-11-10 18:44   ` Noralf Trønnes
2017-11-10 18:44     ` Noralf Trønnes
2017-12-01 14:03   ` Linus Walleij
2017-12-01 14:03     ` Linus Walleij
2017-12-01 18:27     ` Noralf Trønnes
2017-12-01 18:27       ` Noralf Trønnes
2017-12-04  7:45     ` Daniel Vetter
2017-12-04  7:45       ` Daniel Vetter
2017-11-10 12:33 ` [PATCH v1 0/2] DRM driver for ILI9225 display panels Noralf Trønnes
2017-11-10 12:33   ` 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.