Re: [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Den 15.05.2018 23.43, skrev Andy Shevchenko:
On Tue, May 15, 2018 at 4:43 AM, David Lechner <david@xxxxxxxxxxxxxx> wrote:
This adds a new driver for display panels that use the Ilitek ILI9341
controller. It currently supports a single display panel, namely
the YX240QV29-T (e.g. Adafruit 2.4" TFT).

The init sequence is from the Adafruit Python library for the ILI9341
controller. https://github.com/adafruit/Adafruit_Python_ILI9341
Some minor style nitpicks, otherwise LGTM

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

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

diff --git a/MAINTAINERS b/MAINTAINERS
index bc219de9cbee..ffa099abbd79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4480,6 +4480,12 @@ S:       Maintained
  F:     drivers/gpu/drm/tinydrm/ili9225.c
  F:     Documentation/devicetree/bindings/display/ilitek,ili9225.txt

+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M:     David Lechner <david@xxxxxxxxxxxxxx>
+S:     Maintained
+F:     drivers/gpu/drm/tinydrm/ili9341.c
+F:     Documentation/devicetree/bindings/display/ilitek,ili9341.txt
+
  DRM DRIVER FOR INTEL I810 VIDEO CARDS
  S:     Orphan / Obsolete
  F:     drivers/gpu/drm/i810/
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 4592a5e3f20b..7a8008b0783f 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -20,6 +20,16 @@ config TINYDRM_ILI9225

           If M is selected the module will be called ili9225.

+config TINYDRM_ILI9341
+       tristate "DRM support for ILI9341 display panels"
+       depends on DRM_TINYDRM && SPI
Can't we do something like

if SPI

...

endif

?

+       select TINYDRM_MIPI_DBI
+       help
+         DRM driver for the following Ilitek ILI9341 panels:
+         * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
+
+         If M is selected the module will be called ili9341.
+
  config TINYDRM_MI0283QT
         tristate "DRM support for MI0283QT"
         depends on DRM_TINYDRM && SPI
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 49a111929724..14d99080665a 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)          += mipi-dbi.o

  # Displays
  obj-$(CONFIG_TINYDRM_ILI9225)          += ili9225.o
+obj-$(CONFIG_TINYDRM_ILI9341)          += ili9341.o
  obj-$(CONFIG_TINYDRM_MI0283QT)         += mi0283qt.o
  obj-$(CONFIG_TINYDRM_REPAPER)          += repaper.o
  obj-$(CONFIG_TINYDRM_ST7586)           += st7586.o
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
new file mode 100644
index 000000000000..2ce4244a68c3
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <david@xxxxxxxxxxxxxx>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_modeset_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
Can it be in order?

+#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <video/mipi_display.h>
+
+#define ILI9341_FRMCTR1                0xb1
+#define ILI9341_DISCTRL                0xb6
+#define ILI9341_ETMOD          0xb7
+
+#define ILI9341_PWCTRL1                0xc0
+#define ILI9341_PWCTRL2                0xc1
+#define ILI9341_VMCTRL1                0xc5
+#define ILI9341_VMCTRL2                0xc7
+#define ILI9341_PWCTRLA                0xcb
+#define ILI9341_PWCTRLB                0xcf
+
+#define ILI9341_PGAMCTRL       0xe0
+#define ILI9341_NGAMCTRL       0xe1
+#define ILI9341_DTCTRLA                0xe8
+#define ILI9341_DTCTRLB                0xea
+#define ILI9341_PWRSEQ         0xed
+
+#define ILI9341_EN3GAM         0xf2
+#define ILI9341_PUMPCTRL       0xf7
+
+#define ILI9341_MADCTL_BGR     BIT(3)
+#define ILI9341_MADCTL_MV      BIT(5)
+#define ILI9341_MADCTL_MX      BIT(6)
+#define ILI9341_MADCTL_MY      BIT(7)
+
+static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
+                            struct drm_crtc_state *crtc_state,
+                            struct drm_plane_state *plane_state)
+{
+       struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+       struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+       u8 addr_mode;
+       int ret;
+
+       DRM_DEBUG_KMS("\n");
+
+       ret = mipi_dbi_poweron_conditional_reset(mipi);
+       if (ret < 0)
+               return;
+       if (ret == 1)
+               goto out_enable;
+
+       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
+
+       mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+       mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+       mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+       mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+       mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
+       mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
+
+       /* Power Control */
+       mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
+       mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
+       /* VCOM */
+       mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
+       mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
+
+       /* Memory Access Control */
+       mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
+
+       /* Frame Rate */
+       mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
+
+       /* Gamma */
+       mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
+       mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+       mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
+                        0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+                        0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+       mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
+                        0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+                        0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+       /* DDRAM */
+       mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
+
+       /* Display */
+       mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
+       mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+       msleep(100);
+
+       mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+       msleep(100);
+
+out_enable:
+       switch (mipi->rotation) {
+       default:
+               addr_mode = ILI9341_MADCTL_MX;
+               break;
+       case 90:
+               addr_mode = ILI9341_MADCTL_MV;
+               break;
+       case 180:
+               addr_mode = ILI9341_MADCTL_MY;
+               break;
+       case 270:
+               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
+                           ILI9341_MADCTL_MX;
+               break;
+       }
+       addr_mode |= ILI9341_MADCTL_BGR;
+       mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+       mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+}
+
+static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
+       .enable = yx240qv29_enable,
+       .disable = mipi_dbi_pipe_disable,
+       .update = tinydrm_display_pipe_update,
+       .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode yx240qv29_mode = {
+       TINYDRM_MODE(240, 320, 37, 49),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
+
+static struct drm_driver ili9341_driver = {
+       .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
+                                 DRIVER_ATOMIC,
I would rather keep one per line, or all at one line disregard 80
character limit.

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

Reviewed-by: Noralf Trønnes <noralf@xxxxxxxxxxx>

Noralf.

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

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

+};
+MODULE_DEVICE_TABLE(spi, ili9341_id);
+
+static int ili9341_probe(struct spi_device *spi)
+{
+       struct device *dev = &spi->dev;
+       struct mipi_dbi *mipi;
+       struct gpio_desc *dc;
+       u32 rotation = 0;
+       int ret;
+
+       mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+       if (!mipi)
+               return -ENOMEM;
+
+       mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+       if (IS_ERR(mipi->reset)) {
+               DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
+               return PTR_ERR(mipi->reset);
+       }
+
+       dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+       if (IS_ERR(dc)) {
+               DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
+               return PTR_ERR(dc);
+       }
+
+       mipi->regulator = devm_regulator_get(dev, "power");
+       if (IS_ERR(mipi->regulator))
+               return PTR_ERR(mipi->regulator);
+
+       mipi->backlight = devm_of_find_backlight(dev);
+       if (IS_ERR(mipi->backlight))
+               return PTR_ERR(mipi->backlight);
+
+       device_property_read_u32(dev, "rotation", &rotation);

+
+       ret = mipi_dbi_spi_init(spi, mipi, dc);
+       if (ret)
+               return ret;
+
+       ret = mipi_dbi_init(&spi->dev, mipi, &ili9341_pipe_funcs,
+                           &ili9341_driver, &yx240qv29_mode, rotation);
+       if (ret)
+               return ret;
+
+       spi_set_drvdata(spi, mipi);
+
+       return devm_tinydrm_register(&mipi->tinydrm);
+}
+
+static void ili9341_shutdown(struct spi_device *spi)
+{
+       struct mipi_dbi *mipi = spi_get_drvdata(spi);
+
+       tinydrm_shutdown(&mipi->tinydrm);
+}
+
+static struct spi_driver ili9341_spi_driver = {
+       .driver = {
+               .name = "ili9341",
+               .of_match_table = ili9341_of_match,
+       },
+       .id_table = ili9341_id,
+       .probe = ili9341_probe,
+       .shutdown = ili9341_shutdown,
+};
+module_spi_driver(ili9341_spi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
+MODULE_AUTHOR("David Lechner <david@xxxxxxxxxxxxxx>");
+MODULE_LICENSE("GPL");
--
2.17.0




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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux