[PATCH 0/2] Add DRM panel driver for Ilitek ILI9341 based panels in parallel RGB mode

[PATCH 0/2] Add DRM panel driver for Ilitek ILI9341 based panels in parallel RGB mode

[Josef Lusticky]

These patches add panel driver for ili9341-based panels in parallel RGB mode.
The driver was developed for DispleyTech DT024CTFT LCD panel [1] which features ILI9341 chip [2].
The driver was tested on the Allwinner A13 (sun5i) platform.

The driver supports 240x320 pixel resolution with 18-bit RGB (6 wires per color)
and SPI control bus with Data/Command GPIO pin.

The Data/Command GPIO is optional, however at the moment the driver requires it:
The display itself is capable of 9-bit SPI without the Data/Command GPIO.
Support for such configuration can be added later to the driver.

Optional HW reset gpio can be specified, otherwise SW reset command is used
during the initialization.

The ILI9341 displays have two command sets:
Level 1 conforms to MIPI specs
Level 2 is outside the MIPI specs - custom defines are used in the driver

The ILI9341 supports various RGB modes (e.g. NVSYNC, DE_LOW, clock freq, etc.),
however values that are presented in the ILI9341 datasheet [2] are used
by the driver in struct drm_display_mode.


General note on ILI9341-based displays:
The ILI9341 chip can be used in parallel RGB with SPI control
or in SPI only mode where the image data itself is also transferred via SPI.
This driver supports parallel RGB displays - it works with displays wired with 18-bit RGB input,
it does not support SPI data mode (i.e. Multi-inno mi0283qt or Adafruit yx240qv29 are not supported by this driver).
The SPI data mode is naturally much slower than the parallel RGB mode.

General note on DisplayTech DT024CTFT panel:
The panel supports different configuation options (18/16/6-bit RGB or 9/8-bit SPI) depending on the IM[0:3] wiring.
To keep this patch small for reveiw, at the moment only 18-bit RGB mode and 8-bit SPI with Data/Command GPIO
is supported by this driver.


I kindly ask you for a patch review.

Links to datasheet:
[1] https://www.displaytech-us.com/sites/default/files/display-data-sheet/DT024CTFT-v10_0.pdf
[2] https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf

Best regards,
Josef Lusticky

Josef Lusticky (2):
  drm/panel: Add Ilitek ILI9341 panel driver
  dt-bindings: panel: Add Ilitek ILI9341 panel documentation

 .../bindings/display/panel/ilitek,ili9341.txt |  33 ++
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/panel/Kconfig                 |   7 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c  | 320 ++++++++++++++++++
 5 files changed, 367 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c

-- 
2.20.1

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

[PATCH 1/2] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver

[Josef Lusticky]

---
 MAINTAINERS                                  |   6 +
 drivers/gpu/drm/panel/Kconfig                |   7 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 320 +++++++++++++++++++
 4 files changed, 334 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c

diff --git a/MAINTAINERS b/MAINTAINERS
index df5668bc1c88..ec366473174b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4800,6 +4800,12 @@ S:	Maintained
 F:	drivers/gpu/drm/tinydrm/ili9225.c
 F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M:	Josef Lusticky <josef@lusticky.cz>
+S:	Maintained
+F:	drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+F:	Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
+
 DRM DRIVER FOR HX8357D PANELS
 M:	Eric Anholt <eric@anholt.net>
 T:	git git://anongit.freedesktop.org/drm/drm-misc
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3e070153ef21..0e2ceea5009a 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -46,6 +46,13 @@ config DRM_PANEL_ILITEK_IL9322
 	  Say Y here if you want to enable support for Ilitek IL9322
 	  QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
 
+config DRM_PANEL_ILITEK_IL9341
+	tristate "Ilitek ILI9341 240x320 panels"
+	depends on OF && SPI
+	help
+	  Say Y here if you want to enable support for Ilitek IL9341
+	  QVGA (240x320) RGB panel.
+
 config DRM_PANEL_ILITEK_ILI9881C
 	tristate "Ilitek ILI9881C-based panels"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index e7ab71968bbf..1df564018bc4 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
 obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
+obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
 obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
new file mode 100644
index 000000000000..51ed03140f8d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -0,0 +1,320 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Ilitek ILI9341 drm_panel driver
+ * 240RGBx320 dots resolution TFT LCD display
+ *
+ * This driver support the following panel configurations:
+ * - 18-bit parallel RGB interface
+ * - 8-bit SPI with Data/Command GPIO
+ *
+ * Copyright (C) 2019 Josef Lusticky <josef@lusticky.cz>
+ *
+ */
+
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+/* ILI9341 extended register set */
+#define ILI9341_IFMODE         0xB0 // clock polarity
+#define ILI9341_IFCTL          0xF6 // interface conrol
+#define ILI9341_PGAMCTRL       0xE0 // positive gamma control
+#define ILI9341_NGAMCTRL       0xE1 // negative gamma control
+
+#define ILI9341_MADCTL_MV      BIT(5)
+#define ILI9341_MADCTL_MX      BIT(6)
+#define ILI9341_MADCTL_MY      BIT(7)
+
+/**
+ * struct ili9341_config - the display specific configuration
+ * @width_mm: physical panel width [mm]
+ * @height_mm: physical panel height [mm]
+ */
+struct ili9341_config {
+	u32 width_mm;
+	u32 height_mm;
+};
+
+struct ili9341 {
+	const struct ili9341_config *conf;
+	struct drm_panel panel;
+	struct spi_device *spi;
+	struct backlight_device *backlight;
+	struct gpio_desc *dc_gpio;
+	struct gpio_desc *reset_gpio;
+};
+
+static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
+{
+	return container_of(panel, struct ili9341, panel);
+}
+
+static int ili9341_spi_write_command(struct ili9341 *ili, u8 cmd)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = &cmd,
+		.bits_per_word = 8,
+		.len = 1
+	};
+	struct spi_message msg;
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	gpiod_set_value(ili->dc_gpio, 0);
+
+	return spi_sync(ili->spi, &msg);
+}
+
+static int ili9341_spi_write_data(struct ili9341 *ili, u8 *data, size_t size)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = data,
+		.bits_per_word = 8,
+		.len = size
+	};
+
+	struct spi_message msg;
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	gpiod_set_value(ili->dc_gpio, 1);
+
+	return spi_sync(ili->spi, &msg);
+}
+
+#define ili9341_spi_write(ili, cmd, data...) \
+({ \
+	u8 d[] = { data }; \
+	ili9341_spi_write_command(ili, cmd); \
+	if (ARRAY_SIZE(d) > 0) \
+		ili9341_spi_write_data(ili, d, ARRAY_SIZE(d)); \
+})
+
+static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
+{
+	ili9341_spi_write(ili, MIPI_DCS_SET_DISPLAY_OFF);
+	ili9341_spi_write(ili, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(5);
+	return 0;
+}
+
+static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili)
+{
+	/* HW / SW Reset display and wait */
+	if (ili->reset_gpio) {
+		gpiod_set_value(ili->reset_gpio, 0);
+		usleep_range(20, 1000);
+		gpiod_set_value(ili->reset_gpio, 1);
+	} else {
+		ili9341_spi_write(ili, MIPI_DCS_SOFT_RESET);
+	}
+	msleep(120);
+
+	/* Polarity */
+	ili9341_spi_write(ili, ILI9341_IFMODE, 0xC0);
+
+	/* Interface control */
+	ili9341_spi_write(ili, ILI9341_IFCTL, 0x09, 0x01, 0x26);
+
+	/* Pixel format */
+	ili9341_spi_write(ili, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
+
+	/* Gamma */
+	ili9341_spi_write(ili, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	ili9341_spi_write(ili, ILI9341_PGAMCTRL,
+		0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+		0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+	ili9341_spi_write(ili, ILI9341_NGAMCTRL,
+		0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+		0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+	/* Rotation */
+	ili9341_spi_write(ili, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
+
+	/* Exit sleep mode */
+	ili9341_spi_write(ili, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(120);
+
+	ili9341_spi_write(ili, MIPI_DCS_SET_DISPLAY_ON);
+
+	return 0;
+}
+
+static int ili9341_unprepare(struct drm_panel *panel)
+{
+	struct ili9341 *ili = panel_to_ili9341(panel);
+	return ili9341_deinit(panel, ili);
+}
+
+static int ili9341_prepare(struct drm_panel *panel)
+{
+	struct ili9341 *ili = panel_to_ili9341(panel);
+	int ret;
+
+	ret = ili9341_init(panel, ili);
+	if (ret < 0)
+		ili9341_unprepare(panel);
+	return ret;
+}
+
+static int ili9341_enable(struct drm_panel *panel)
+{
+	struct ili9341 *ili = panel_to_ili9341(panel);
+	return backlight_enable(ili->backlight);
+}
+
+static int ili9341_disable(struct drm_panel *panel)
+{
+	struct ili9341 *ili = panel_to_ili9341(panel);
+	return backlight_disable(ili->backlight);
+}
+
+static const struct drm_display_mode prgb_240x320_mode = {
+	.clock = 6350,
+
+	.hdisplay = 240,
+	.hsync_start = 240 + 10,
+	.hsync_end = 240 + 10 + 10,
+	.htotal = 240 + 10 + 10 + 20,
+
+	.vdisplay = 320,
+	.vsync_start = 320 + 4,
+	.vsync_end = 320 + 4 + 2,
+	.vtotal = 320 + 4 + 2 + 2,
+
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED
+};
+
+static int ili9341_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct ili9341 *ili = panel_to_ili9341(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &prgb_240x320_mode);
+	drm_mode_set_name(mode);
+
+	mode->width_mm = ili->conf->width_mm;
+	mode->height_mm = ili->conf->height_mm;
+
+	strncpy(connector->display_info.name, "ILI9341 TFT LCD driver\0",
+		DRM_DISPLAY_INFO_LEN);
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	connector->display_info.bus_flags |= DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
+		DRM_BUS_FLAG_SYNC_NEGEDGE;
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1; /* Number of modes */
+}
+
+static const struct drm_panel_funcs ili9341_drm_funcs = {
+	.disable = ili9341_disable,
+	.unprepare = ili9341_unprepare,
+	.prepare = ili9341_prepare,
+	.enable = ili9341_enable,
+	.get_modes = ili9341_get_modes,
+};
+
+static int ili9341_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct ili9341 *ili;
+	int ret;
+
+	ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL);
+	if (!ili)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, ili);
+
+	ili->spi = spi;
+
+	/*
+	 * Every new incarnation of this display must have a unique
+	 * data entry for the system in this driver.
+	 */
+	ili->conf = of_device_get_match_data(dev);
+	if (!ili->conf) {
+		DRM_DEV_ERROR(dev, "missing device configuration\n");
+		return -ENODEV;
+	}
+
+	ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ili->reset_gpio)) {
+		DRM_DEV_ERROR(dev, "failed to get reset gpio\n");
+		return PTR_ERR(ili->reset_gpio);
+	}
+
+	ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(ili->dc_gpio)) {
+		DRM_DEV_ERROR(dev, "failed to get dc gpio\n");
+		return PTR_ERR(ili->dc_gpio);
+	}
+
+	ili->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(ili->backlight)) {
+		DRM_DEV_ERROR(dev, "failed to get backlight\n");
+		return PTR_ERR(ili->backlight);
+	}
+
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "spi setup failed\n");
+		return ret;
+	}
+
+	drm_panel_init(&ili->panel);
+	ili->panel.dev = dev;
+	ili->panel.funcs = &ili9341_drm_funcs;
+
+	return drm_panel_add(&ili->panel);
+}
+
+static int ili9341_remove(struct spi_device *spi)
+{
+	struct ili9341 *ili = spi_get_drvdata(spi);
+
+	drm_panel_remove(&ili->panel);
+
+	ili9341_disable(&ili->panel);
+	ili9341_unprepare(&ili->panel);
+
+	return 0;
+}
+
+static const struct ili9341_config dt024ctft_data = {
+	.width_mm = 37,
+	.height_mm = 49,
+};
+
+static const struct of_device_id ili9341_of_match[] = {
+	{ .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ili9341_of_match);
+
+static struct spi_driver ili9341_driver = {
+	.probe = ili9341_probe,
+	.remove = ili9341_remove,
+	.driver = {
+		.name = "panel-ilitek-ili9341",
+		.of_match_table = ili9341_of_match,
+	},
+};
+module_spi_driver(ili9341_driver);
+
+MODULE_AUTHOR("Josef Lusticky <josef@lusticky.cz>");
+MODULE_DESCRIPTION("ILI9341 LCD panel driver");
+MODULE_LICENSE("GPL");
-- 
2.20.1

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

[PATCH 2/2] dt-bindings: panel: Add Ilitek ILI9341 panel documentation

[Josef Lusticky]

---
 .../bindings/display/panel/ilitek,ili9341.txt | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
new file mode 100644
index 000000000000..4e0e483bc12e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
@@ -0,0 +1,33 @@
+Ilitek ILI9341 TFT panel driver with SPI control bus
+
+This is a driver for 240x320 TFT panels with parallel RGB color input.
+
+Required properties:
+  - compatible: "displaytech,dt024ctft", "ilitek,ili9341"
+  - backlight: phandle of the backlight device attached to the panel
+
+Optional properties:
+  - dc-gpios: a GPIO spec for the Data/Command pin, see gpio/gpio.txt
+  - reset-gpios: a GPIO spec for the reset pin, see gpio/gpio.txt
+
+The panel must obey the rules for a SPI slave device as specified in
+spi/spi-bus.txt
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in
+media/video-interfaces.txt. This node should describe panel's video bus.
+
+Example:
+
+panel@0 {
+	compatible = "displaytech,dt024ctft", "ilitek,ili9341";
+	reg = <0>;
+	backlight = <&backlight>;
+	dc-gpios = <&pio 4 9 GPIO_ACTIVE_HIGH>;
+
+	port {
+		panel_in: endpoint {
+			remote-endpoint = <&display_out>;
+		};
+	};
+};
-- 
2.20.1

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

Re: [PATCH 2/2] dt-bindings: panel: Add Ilitek ILI9341 panel documentation

[Sam Ravnborg]

Hi Josef.

Please follow guidelines in
Documentation/devicetree/bindings/submitting...

You will among other learn to send this patch to the right mailing list.
And please add a short summary in the changelog body.
The changelog body shall not require the reader to rtead subject to know
what the patch contains.

In next revisions, sent to the right list etc. you can add my:

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>


On Tue, Feb 19, 2019 at 02:21:01PM +0100, Josef Lusticky wrote:
> ---
>  .../bindings/display/panel/ilitek,ili9341.txt | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
> new file mode 100644
> index 000000000000..4e0e483bc12e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
> @@ -0,0 +1,33 @@
> +Ilitek ILI9341 TFT panel driver with SPI control bus
> +
> +This is a driver for 240x320 TFT panels with parallel RGB color input.
> +
> +Required properties:
> +  - compatible: "displaytech,dt024ctft", "ilitek,ili9341"
> +  - backlight: phandle of the backlight device attached to the panel
> +
> +Optional properties:
> +  - dc-gpios: a GPIO spec for the Data/Command pin, see gpio/gpio.txt
> +  - reset-gpios: a GPIO spec for the reset pin, see gpio/gpio.txt
> +
> +The panel must obey the rules for a SPI slave device as specified in
> +spi/spi-bus.txt
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in
> +media/video-interfaces.txt. This node should describe panel's video bus.
> +
> +Example:
> +
> +panel@0 {
> +	compatible = "displaytech,dt024ctft", "ilitek,ili9341";
> +	reg = <0>;
> +	backlight = <&backlight>;
> +	dc-gpios = <&pio 4 9 GPIO_ACTIVE_HIGH>;
> +
> +	port {
> +		panel_in: endpoint {
> +			remote-endpoint = <&display_out>;
> +		};
> +	};
> +};
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver

[Sam Ravnborg]

Hi Josef.

Thanks for the patch. A little late review follows.

Just a few thing, but please fix and resend.

	Sam

> +++ b/MAINTAINERS
> @@ -4800,6 +4800,12 @@ S:	Maintained
>  F:	drivers/gpu/drm/tinydrm/ili9225.c
>  F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>  
> +DRM DRIVER FOR ILITEK ILI9341 PANELS
> +M:	Josef Lusticky <josef@lusticky.cz>
> +S:	Maintained
> +F:	drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> +F:	Documentation/devicetree/bindings/display/panel/ilitek,ili9341.txt
> +
>  DRM DRIVER FOR HX8357D PANELS
>  M:	Eric Anholt <eric@anholt.net>
>  T:	git git://anongit.freedesktop.org/drm/drm-misc

I recall entries in MAINTAINERS are supposed to be ordred alphabitically
after their title. So this needs to be fixed (H comes before I)

>  
> +config DRM_PANEL_ILITEK_IL9341
> +	tristate "Ilitek ILI9341 240x320 panels"
> +	depends on OF && SPI
> +	help
> +	  Say Y here if you want to enable support for Ilitek IL9341
> +	  QVGA (240x320) RGB panel.
The panel uses backlight - should it depens on BACKLIGHT?

> +
>  config DRM_PANEL_ILITEK_ILI9881C
>  	tristate "Ilitek ILI9881C-based panels"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index e7ab71968bbf..1df564018bc4 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
>  obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
> +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> new file mode 100644
> index 000000000000..51ed03140f8d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> @@ -0,0 +1,320 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Ilitek ILI9341 drm_panel driver
> + * 240RGBx320 dots resolution TFT LCD display
> + *
> + * This driver support the following panel configurations:
> + * - 18-bit parallel RGB interface
> + * - 8-bit SPI with Data/Command GPIO
> + *
> + * Copyright (C) 2019 Josef Lusticky <josef@lusticky.cz>
> + *
> + */
> +
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/spi/spi.h>
> +
> +#include <video/mipi_display.h>

Please include files in following blocks:

#include <linux/*>

#include <video/*>

#include <drm/*>

This matches what other panel drivers do today.
(It was only recently made the same for all drivers)


> +
> +/* ILI9341 extended register set */
> +#define ILI9341_IFMODE         0xB0 // clock polarity
> +#define ILI9341_IFCTL          0xF6 // interface conrol
> +#define ILI9341_PGAMCTRL       0xE0 // positive gamma control
> +#define ILI9341_NGAMCTRL       0xE1 // negative gamma control
> +
> +#define ILI9341_MADCTL_MV      BIT(5)
> +#define ILI9341_MADCTL_MX      BIT(6)
> +#define ILI9341_MADCTL_MY      BIT(7)
> +
> +/**
> + * struct ili9341_config - the display specific configuration
> + * @width_mm: physical panel width [mm]
> + * @height_mm: physical panel height [mm]
> + */
> +struct ili9341_config {
> +	u32 width_mm;
> +	u32 height_mm;
> +};
> +
> +struct ili9341 {
> +	const struct ili9341_config *conf;
> +	struct drm_panel panel;
> +	struct spi_device *spi;
> +	struct backlight_device *backlight;
> +	struct gpio_desc *dc_gpio;
> +	struct gpio_desc *reset_gpio;
> +};
No power-supply?
It is not in the binding, so likely not. But just wondering.

> +
> +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ili9341, panel);
> +}
> +
> +static int ili9341_spi_write_command(struct ili9341 *ili, u8 cmd)
> +{
> +	struct spi_transfer xfer = {
> +		.tx_buf = &cmd,
> +		.bits_per_word = 8,
> +		.len = 1
> +	};
> +	struct spi_message msg;
> +	spi_message_init(&msg);
> +	spi_message_add_tail(&xfer, &msg);
> +
> +	gpiod_set_value(ili->dc_gpio, 0);
> +
> +	return spi_sync(ili->spi, &msg);
> +}
> +
> +static int ili9341_spi_write_data(struct ili9341 *ili, u8 *data, size_t size)
> +{
> +	struct spi_transfer xfer = {
> +		.tx_buf = data,
> +		.bits_per_word = 8,
> +		.len = size
> +	};
> +
> +	struct spi_message msg;
> +	spi_message_init(&msg);
> +	spi_message_add_tail(&xfer, &msg);
> +
> +	gpiod_set_value(ili->dc_gpio, 1);
> +
> +	return spi_sync(ili->spi, &msg);
> +}
> +
> +#define ili9341_spi_write(ili, cmd, data...) \
> +({ \
> +	u8 d[] = { data }; \
> +	ili9341_spi_write_command(ili, cmd); \
> +	if (ARRAY_SIZE(d) > 0) \
> +		ili9341_spi_write_data(ili, d, ARRAY_SIZE(d)); \
> +})
> +
> +static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
> +{
> +	ili9341_spi_write(ili, MIPI_DCS_SET_DISPLAY_OFF);
> +	ili9341_spi_write(ili, MIPI_DCS_ENTER_SLEEP_MODE);
> +	msleep(5);
> +	return 0;
> +}
> +
> +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili)
> +{
> +	/* HW / SW Reset display and wait */
> +	if (ili->reset_gpio) {
> +		gpiod_set_value(ili->reset_gpio, 0);
> +		usleep_range(20, 1000);
> +		gpiod_set_value(ili->reset_gpio, 1);
> +	} else {
> +		ili9341_spi_write(ili, MIPI_DCS_SOFT_RESET);
> +	}
> +	msleep(120);
> +
> +	/* Polarity */
> +	ili9341_spi_write(ili, ILI9341_IFMODE, 0xC0);
> +
> +	/* Interface control */
> +	ili9341_spi_write(ili, ILI9341_IFCTL, 0x09, 0x01, 0x26);
> +
> +	/* Pixel format */
> +	ili9341_spi_write(ili, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
> +
> +	/* Gamma */
> +	ili9341_spi_write(ili, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> +	ili9341_spi_write(ili, ILI9341_PGAMCTRL,
> +		0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
> +		0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> +	ili9341_spi_write(ili, ILI9341_NGAMCTRL,
> +		0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
> +		0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
> +
> +	/* Rotation */
> +	ili9341_spi_write(ili, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
> +
> +	/* Exit sleep mode */
> +	ili9341_spi_write(ili, MIPI_DCS_EXIT_SLEEP_MODE);
> +	msleep(120);
> +
> +	ili9341_spi_write(ili, MIPI_DCS_SET_DISPLAY_ON);
> +
> +	return 0;
> +}
> +
> +static int ili9341_unprepare(struct drm_panel *panel)
> +{
> +	struct ili9341 *ili = panel_to_ili9341(panel);
> +	return ili9341_deinit(panel, ili);
> +}
> +
> +static int ili9341_prepare(struct drm_panel *panel)
> +{
> +	struct ili9341 *ili = panel_to_ili9341(panel);
> +	int ret;
> +
> +	ret = ili9341_init(panel, ili);
> +	if (ret < 0)
> +		ili9341_unprepare(panel);
> +	return ret;
> +}
> +
> +static int ili9341_enable(struct drm_panel *panel)
> +{
> +	struct ili9341 *ili = panel_to_ili9341(panel);
> +	return backlight_enable(ili->backlight);
> +}
> +
> +static int ili9341_disable(struct drm_panel *panel)
> +{
> +	struct ili9341 *ili = panel_to_ili9341(panel);
> +	return backlight_disable(ili->backlight);
> +}
> +
> +static const struct drm_display_mode prgb_240x320_mode = {
> +	.clock = 6350,
> +
> +	.hdisplay = 240,
> +	.hsync_start = 240 + 10,
> +	.hsync_end = 240 + 10 + 10,
> +	.htotal = 240 + 10 + 10 + 20,
> +
> +	.vdisplay = 320,
> +	.vsync_start = 320 + 4,
> +	.vsync_end = 320 + 4 + 2,
> +	.vtotal = 320 + 4 + 2 + 2,
> +
> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED
> +};
> +
> +static int ili9341_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct ili9341 *ili = panel_to_ili9341(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &prgb_240x320_mode);
check if drm_mode_duplicate() fails, and handle this.
See how other drivers do it.

> +	drm_mode_set_name(mode);
> +
> +	mode->width_mm = ili->conf->width_mm;
> +	mode->height_mm = ili->conf->height_mm;
> +
> +	strncpy(connector->display_info.name, "ILI9341 TFT LCD driver\0",
> +		DRM_DISPLAY_INFO_LEN);
> +	connector->display_info.width_mm = mode->width_mm;
> +	connector->display_info.height_mm = mode->height_mm;
> +	connector->display_info.bus_flags |= DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
> +		DRM_BUS_FLAG_SYNC_NEGEDGE;
> +
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1; /* Number of modes */
> +}
> +
> +static const struct drm_panel_funcs ili9341_drm_funcs = {
> +	.disable = ili9341_disable,
> +	.unprepare = ili9341_unprepare,
> +	.prepare = ili9341_prepare,
> +	.enable = ili9341_enable,
> +	.get_modes = ili9341_get_modes,
> +};
> +
> +static int ili9341_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct ili9341 *ili;
> +	int ret;
> +
> +	ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL);
> +	if (!ili)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, ili);
> +
> +	ili->spi = spi;
> +
> +	/*
> +	 * Every new incarnation of this display must have a unique
> +	 * data entry for the system in this driver.
> +	 */
> +	ili->conf = of_device_get_match_data(dev);
> +	if (!ili->conf) {
> +		DRM_DEV_ERROR(dev, "missing device configuration\n");
> +		return -ENODEV;
> +	}
> +
> +	ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ili->reset_gpio)) {
> +		DRM_DEV_ERROR(dev, "failed to get reset gpio\n");
> +		return PTR_ERR(ili->reset_gpio);
> +	}
> +
> +	ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
> +	if (IS_ERR(ili->dc_gpio)) {
> +		DRM_DEV_ERROR(dev, "failed to get dc gpio\n");
> +		return PTR_ERR(ili->dc_gpio);
> +	}
> +
> +	ili->backlight = devm_of_find_backlight(dev);
> +	if (IS_ERR(ili->backlight)) {
> +		DRM_DEV_ERROR(dev, "failed to get backlight\n");
> +		return PTR_ERR(ili->backlight);
> +	}
> +
> +	ret = spi_setup(spi);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev, "spi setup failed\n");
> +		return ret;
> +	}
> +
> +	drm_panel_init(&ili->panel);
> +	ili->panel.dev = dev;
> +	ili->panel.funcs = &ili9341_drm_funcs;
> +
> +	return drm_panel_add(&ili->panel);
> +}
> +
> +static int ili9341_remove(struct spi_device *spi)
> +{
> +	struct ili9341 *ili = spi_get_drvdata(spi);
> +
> +	drm_panel_remove(&ili->panel);
> +
> +	ili9341_disable(&ili->panel);
> +	ili9341_unprepare(&ili->panel);
> +
> +	return 0;
> +}
> +
> +static const struct ili9341_config dt024ctft_data = {
> +	.width_mm = 37,
> +	.height_mm = 49,
> +};
> +
> +static const struct of_device_id ili9341_of_match[] = {
> +	{ .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data },
> +	{ }
Many drivers add the comment { /* sentinel */ } to document this is a
mandatory end marker.

> +};
> +MODULE_DEVICE_TABLE(of, ili9341_of_match);
> +
> +static struct spi_driver ili9341_driver = {
> +	.probe = ili9341_probe,
> +	.remove = ili9341_remove,
> +	.driver = {
> +		.name = "panel-ilitek-ili9341",
> +		.of_match_table = ili9341_of_match,
> +	},
> +};
> +module_spi_driver(ili9341_driver);
> +
> +MODULE_AUTHOR("Josef Lusticky <josef@lusticky.cz>");
> +MODULE_DESCRIPTION("ILI9341 LCD panel driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] Add DRM panel driver for Ilitek ILI9341 based panels in parallel RGB mode

[Sam Ravnborg]

Hi Josef.

On Tue, Feb 19, 2019 at 02:20:59PM +0100, Josef Lusticky wrote:
> These patches add panel driver for ili9341-based panels in parallel RGB mode.
> The driver was developed for DispleyTech DT024CTFT LCD panel [1] which features ILI9341 chip [2].
> The driver was tested on the Allwinner A13 (sun5i) platform.
> 
> The driver supports 240x320 pixel resolution with 18-bit RGB (6 wires per color)
> and SPI control bus with Data/Command GPIO pin.
> 
> The Data/Command GPIO is optional, however at the moment the driver requires it:
> The display itself is capable of 9-bit SPI without the Data/Command GPIO.
> Support for such configuration can be added later to the driver.
> 
> Optional HW reset gpio can be specified, otherwise SW reset command is used
> during the initialization.
> 
> The ILI9341 displays have two command sets:
> Level 1 conforms to MIPI specs
> Level 2 is outside the MIPI specs - custom defines are used in the driver
> 
> The ILI9341 supports various RGB modes (e.g. NVSYNC, DE_LOW, clock freq, etc.),
> however values that are presented in the ILI9341 datasheet [2] are used
> by the driver in struct drm_display_mode.
> 
> 
> General note on ILI9341-based displays:
> The ILI9341 chip can be used in parallel RGB with SPI control
> or in SPI only mode where the image data itself is also transferred via SPI.
> This driver supports parallel RGB displays - it works with displays wired with 18-bit RGB input,
> it does not support SPI data mode (i.e. Multi-inno mi0283qt or Adafruit yx240qv29 are not supported by this driver).
> The SPI data mode is naturally much slower than the parallel RGB mode.
> 
> General note on DisplayTech DT024CTFT panel:
> The panel supports different configuation options (18/16/6-bit RGB or 9/8-bit SPI) depending on the IM[0:3] wiring.
> To keep this patch small for reveiw, at the moment only 18-bit RGB mode and 8-bit SPI with Data/Command GPIO
> is supported by this driver.
> 
> 
> I kindly ask you for a patch review.
Did you get around to send a v2 of this patchset?

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

Re: [PATCH 0/2] Add DRM panel driver for Ilitek ILI9341 based panels in parallel RGB mode

[Josef Luštický]

Hello Sam, Rob,
thank you both for comments.

I've already prepared most of the code for v2 of this patchset.

At the moment I am investigating, if it's possible to use mipi_dbi_*
functions for the display initialization,
as Rob suggested ("Can't you use existing mipi_dbi_* functions instead
of rolling your own?").

I'll post v2 patchset soon.

Kind regards
Josef

st 26. 6. 2019 v 14:57 odesílatel Sam Ravnborg <sam@ravnborg.org> napsal:
>
> Hi Josef.
>
> On Tue, Feb 19, 2019 at 02:20:59PM +0100, Josef Lusticky wrote:
> > These patches add panel driver for ili9341-based panels in parallel RGB mode.
> > The driver was developed for DispleyTech DT024CTFT LCD panel [1] which features ILI9341 chip [2].
> > The driver was tested on the Allwinner A13 (sun5i) platform.
> >
> > The driver supports 240x320 pixel resolution with 18-bit RGB (6 wires per color)
> > and SPI control bus with Data/Command GPIO pin.
> >
> > The Data/Command GPIO is optional, however at the moment the driver requires it:
> > The display itself is capable of 9-bit SPI without the Data/Command GPIO.
> > Support for such configuration can be added later to the driver.
> >
> > Optional HW reset gpio can be specified, otherwise SW reset command is used
> > during the initialization.
> >
> > The ILI9341 displays have two command sets:
> > Level 1 conforms to MIPI specs
> > Level 2 is outside the MIPI specs - custom defines are used in the driver
> >
> > The ILI9341 supports various RGB modes (e.g. NVSYNC, DE_LOW, clock freq, etc.),
> > however values that are presented in the ILI9341 datasheet [2] are used
> > by the driver in struct drm_display_mode.
> >
> >
> > General note on ILI9341-based displays:
> > The ILI9341 chip can be used in parallel RGB with SPI control
> > or in SPI only mode where the image data itself is also transferred via SPI.
> > This driver supports parallel RGB displays - it works with displays wired with 18-bit RGB input,
> > it does not support SPI data mode (i.e. Multi-inno mi0283qt or Adafruit yx240qv29 are not supported by this driver).
> > The SPI data mode is naturally much slower than the parallel RGB mode.
> >
> > General note on DisplayTech DT024CTFT panel:
> > The panel supports different configuation options (18/16/6-bit RGB or 9/8-bit SPI) depending on the IM[0:3] wiring.
> > To keep this patch small for reveiw, at the moment only 18-bit RGB mode and 8-bit SPI with Data/Command GPIO
> > is supported by this driver.
> >
> >
> > I kindly ask you for a patch review.
> Did you get around to send a v2 of this patchset?
>
>         Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] Add DRM panel driver for Ilitek ILI9341 based panels in parallel RGB mode

[Sam Ravnborg]

Hi Josef.

On Wed, Jun 26, 2019 at 05:21:09PM +0200, Josef Luštický wrote:
> Hello Sam, Rob,
> thank you both for comments.
> 
> I've already prepared most of the code for v2 of this patchset.
> 
> At the moment I am investigating, if it's possible to use mipi_dbi_*
> functions for the display initialization,
> as Rob suggested ("Can't you use existing mipi_dbi_* functions instead
> of rolling your own?").
> 
> I'll post v2 patchset soon.
Thanks for the update. Looks forward for next round of patches.

Please check also what we have in drm/tinydrm/core/tinydrm-helpers.c.
If some of the functions are useable outside tinydrm - then we should
move them to a helper in drm/.
So we do not duplicate functionality.

And maybe this is what you already do and so everything is fine.

Do not hesitate to get back if you have any questions.

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