All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/4] drm/mipi-dbi: Support panel drivers
@ 2019-07-29 19:55 Noralf Trønnes
  2019-07-29 19:55 ` [RFC 1/4] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver Noralf Trønnes
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-29 19:55 UTC (permalink / raw)
  To: dri-devel; +Cc: daniel.vetter, josef, thierry.reding, laurent.pinchart, sam

Inspired by the thread[1] following the submission of a new ili9341
panel driver[2], I set out to see if I could support panel drivers in
drm_mipi_dbi.

I have included the original driver, done some prep work on it, added
panel support to drm_mipi_dbi and converted mi0283qt to this new panel
driver.

The big question is whether or not panel drivers should be allowed to
turn themselves into full fledged DRM drivers.

Noralf.

[1]
https://lists.freedesktop.org/archives/dri-devel/2019-July/228193.html
[2] https://patchwork.freedesktop.org/patch/316528/

Josef Lusticky (1):
  drm/panel: Add Ilitek ILI9341 parallel RGB panel driver

Noralf Trønnes (3):
  drm/panel/ili9341: Rebase and some more
  drm/mipi-dbi: Support command mode panel drivers
  drm/panel/ili9341: Support mi0283qt

 MAINTAINERS                                  |   6 +
 drivers/gpu/drm/drm_mipi_dbi.c               | 110 +++++
 drivers/gpu/drm/panel/Kconfig                |   9 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 452 +++++++++++++++++++
 include/drm/drm_mipi_dbi.h                   |   8 +
 6 files changed, 586 insertions(+)
 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

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

* [RFC 1/4] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver
  2019-07-29 19:55 [RFC 0/4] drm/mipi-dbi: Support panel drivers Noralf Trønnes
@ 2019-07-29 19:55 ` Noralf Trønnes
  2019-07-29 19:55 ` [RFC 2/4] drm/panel/ili9341: Rebase and some more Noralf Trønnes
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-29 19:55 UTC (permalink / raw)
  To: dri-devel; +Cc: daniel.vetter, josef, thierry.reding, laurent.pinchart, sam

From: Josef Lusticky <josef@lusticky.cz>

Add driver for Ilitek ILI9341 panels in parallel RGB mode

Signed-off-by: Josef Lusticky <josef@lusticky.cz>
---
 MAINTAINERS                                  |   6 +
 drivers/gpu/drm/panel/Kconfig                |   9 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 291 +++++++++++++++++++
 4 files changed, 307 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 060ffe635832..1f638d96113b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5115,6 +5115,12 @@ S:	Maintained
 F:	drivers/gpu/drm/tinydrm/hx8357d.c
 F:	Documentation/devicetree/bindings/display/himax,hx8357d.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 INTEL I810 VIDEO CARDS
 S:	Orphan / Obsolete
 F:	drivers/gpu/drm/i810/
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index eaecd40cc32e..34a5b262f924 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -56,6 +56,15 @@ 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_ILI9341
+	tristate "Ilitek ILI9341 240x320 panels"
+	depends on OF && SPI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select TINYDRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for Ilitek ILI9341
+	  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 62dae45f8f74..ba4a303c1a66 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
+obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += 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..0c700b171025
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Ilitek ILI9341 drm_panel driver
+ * 240RGBx320 dots resolution TFT LCD display
+ *
+ * This driver supports the following panel configurations:
+ * - Command interface:
+ *     - MIPI-DBI Type 3 Option 1
+ *       (9-bit SPI with Data/Command bit  - IM[3:0] = 1101)
+ *     - MIPI-DBI Type 3 Option 3
+ *       (8-bit SPI with Data/Command GPIO - IM[3:0] = 1110)
+ * - Graphical data interface:
+ *     - MIPI-DPI 18-bit parallel RGB interface
+ *
+ * Copyright (C) 2019 Josef Lusticky <josef@lusticky.cz>
+ *
+ */
+
+#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>
+
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+#include <drm/tinydrm/mipi-dbi.h>
+
+/* ILI9341 extended register set (Vendor Command 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 {
+	struct drm_panel panel;
+	struct mipi_dbi *mipi;
+	const struct ili9341_config *conf;
+};
+
+static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
+{
+	return container_of(panel, struct ili9341, panel);
+}
+
+static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
+{
+	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_OFF);
+	mipi_dbi_command(ili->mipi, 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->mipi->reset)
+		mipi_dbi_hw_reset(ili->mipi);
+
+	mipi_dbi_command(ili->mipi, MIPI_DCS_SOFT_RESET);
+	msleep(120);
+
+	/* Polarity */
+	mipi_dbi_command(ili->mipi, ILI9341_IFMODE, 0xC0);
+
+	/* Interface control */
+	mipi_dbi_command(ili->mipi, ILI9341_IFCTL, 0x09, 0x01, 0x26);
+
+	/* Pixel format */
+	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
+
+	/* Gamma */
+	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	mipi_dbi_command(ili->mipi, ILI9341_PGAMCTRL,
+		0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+		0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+	mipi_dbi_command(ili->mipi, ILI9341_NGAMCTRL,
+		0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+		0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+	/* Rotation */
+	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
+
+	/* Exit sleep mode */
+	mipi_dbi_command(ili->mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(120);
+
+	mipi_dbi_command(ili->mipi, 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->mipi->backlight);
+}
+
+static int ili9341_disable(struct drm_panel *panel)
+{
+	struct ili9341 *ili = panel_to_ili9341(panel);
+
+	return backlight_disable(ili->mipi->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);
+	if (!mode) {
+		DRM_DEV_ERROR(panel->drm->dev, "bad mode or failed to add mode\n");
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->width_mm = ili->conf->width_mm;
+	mode->height_mm = ili->conf->height_mm;
+
+	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;
+	struct mipi_dbi *mipi;
+	struct gpio_desc *dc_gpio;
+	int ret;
+
+	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	if (!mipi)
+		return -ENOMEM;
+
+	ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL);
+	if (!ili)
+		return -ENOMEM;
+
+	ili->mipi = mipi;
+
+	spi_set_drvdata(spi, ili);
+
+	/*
+	 * 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->mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ili->mipi->reset)) {
+		DRM_DEV_ERROR(dev, "failed to get gpio 'reset'\n");
+		return PTR_ERR(ili->mipi->reset);
+	}
+
+	ili->mipi->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(ili->mipi->backlight)) {
+		DRM_DEV_ERROR(dev, "failed to get backlight\n");
+		return PTR_ERR(ili->mipi->backlight);
+	}
+
+	dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc_gpio)) {
+		DRM_DEV_ERROR(dev, "failed to get gpio 'dc'\n");
+		return PTR_ERR(dc_gpio);
+	}
+
+	ret = mipi_dbi_spi_init(spi, ili->mipi, dc_gpio);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "MIPI-DBI 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 },
+	{ /* sentinel */ }
+};
+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

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

* [RFC 2/4] drm/panel/ili9341: Rebase and some more
  2019-07-29 19:55 [RFC 0/4] drm/mipi-dbi: Support panel drivers Noralf Trønnes
  2019-07-29 19:55 ` [RFC 1/4] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver Noralf Trønnes
@ 2019-07-29 19:55 ` Noralf Trønnes
  2019-07-30  6:45   ` Josef Luštický
  2019-07-29 19:55 ` [RFC 3/4] drm/mipi-dbi: Support command mode panel drivers Noralf Trønnes
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-29 19:55 UTC (permalink / raw)
  To: dri-devel; +Cc: daniel.vetter, josef, thierry.reding, laurent.pinchart, sam

Embed mipi_dbi in struct ili9341.
Rebase on moved mipi-dbi, rename variable name.
Add backlight_device to panel struct.
mipi_dbi_hw_reset() already has a NULL check on the reset gpio.
Prepare for more panels by reworking ili9341_config.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 104 +++++++++----------
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 0c700b171025..a755f3e60111 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -28,7 +28,7 @@
 #include <drm/drm_modes.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_print.h>
-#include <drm/tinydrm/mipi-dbi.h>
+#include <drm/drm_mipi_dbi.h>
 
 /* ILI9341 extended register set (Vendor Command Set) */
 #define ILI9341_IFMODE         0xB0 // clock polarity
@@ -42,17 +42,18 @@
 
 /**
  * struct ili9341_config - the display specific configuration
- * @width_mm: physical panel width [mm]
- * @height_mm: physical panel height [mm]
+ * @funcs: Panel functions
+ * @mode: Display mode
  */
 struct ili9341_config {
-	u32 width_mm;
-	u32 height_mm;
+	const struct drm_panel_funcs *funcs;
+	const struct drm_display_mode *mode;
 };
 
 struct ili9341 {
 	struct drm_panel panel;
-	struct mipi_dbi *mipi;
+	struct mipi_dbi dbi;
+	struct backlight_device *backlight;
 	const struct ili9341_config *conf;
 };
 
@@ -63,47 +64,50 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
 
 static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
 {
-	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_OFF);
-	mipi_dbi_command(ili->mipi, MIPI_DCS_ENTER_SLEEP_MODE);
+	struct mipi_dbi *dbi = &ili->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
 	msleep(5);
 	return 0;
 }
 
-static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili)
+static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili)
 {
-	/* HW / SW Reset display and wait */
-	if (ili->mipi->reset)
-		mipi_dbi_hw_reset(ili->mipi);
+	struct mipi_dbi *dbi = &ili->dbi;
 
-	mipi_dbi_command(ili->mipi, MIPI_DCS_SOFT_RESET);
+	/* HW / SW Reset display and wait */
+	mipi_dbi_hw_reset(dbi);
+
+	mipi_dbi_command(dbi, MIPI_DCS_SOFT_RESET);
 	msleep(120);
 
 	/* Polarity */
-	mipi_dbi_command(ili->mipi, ILI9341_IFMODE, 0xC0);
+	mipi_dbi_command(dbi, ILI9341_IFMODE, 0xC0);
 
 	/* Interface control */
-	mipi_dbi_command(ili->mipi, ILI9341_IFCTL, 0x09, 0x01, 0x26);
+	mipi_dbi_command(dbi, ILI9341_IFCTL, 0x09, 0x01, 0x26);
 
 	/* Pixel format */
-	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
 
 	/* Gamma */
-	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
-	mipi_dbi_command(ili->mipi, ILI9341_PGAMCTRL,
+	mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	mipi_dbi_command(dbi, ILI9341_PGAMCTRL,
 		0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
 		0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
-	mipi_dbi_command(ili->mipi, ILI9341_NGAMCTRL,
+	mipi_dbi_command(dbi, ILI9341_NGAMCTRL,
 		0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
 		0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
 
 	/* Rotation */
-	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
 
 	/* Exit sleep mode */
-	mipi_dbi_command(ili->mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
 	msleep(120);
 
-	mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_ON);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
 
 	return 0;
 }
@@ -115,12 +119,12 @@ static int ili9341_unprepare(struct drm_panel *panel)
 	return ili9341_deinit(panel, ili);
 }
 
-static int ili9341_prepare(struct drm_panel *panel)
+static int dt024ctft_prepare(struct drm_panel *panel)
 {
 	struct ili9341 *ili = panel_to_ili9341(panel);
 	int ret;
 
-	ret = ili9341_init(panel, ili);
+	ret = dt024ctft_init(panel, ili);
 	if (ret < 0)
 		ili9341_unprepare(panel);
 	return ret;
@@ -130,14 +134,14 @@ static int ili9341_enable(struct drm_panel *panel)
 {
 	struct ili9341 *ili = panel_to_ili9341(panel);
 
-	return backlight_enable(ili->mipi->backlight);
+	return backlight_enable(ili->backlight);
 }
 
 static int ili9341_disable(struct drm_panel *panel)
 {
 	struct ili9341 *ili = panel_to_ili9341(panel);
 
-	return backlight_disable(ili->mipi->backlight);
+	return backlight_disable(ili->backlight);
 }
 
 static const struct drm_display_mode prgb_240x320_mode = {
@@ -154,7 +158,10 @@ static const struct drm_display_mode prgb_240x320_mode = {
 	.vtotal = 320 + 4 + 2 + 2,
 
 	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
-	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED
+	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+
+	.width_mm = 37,
+	.height_mm = 49,
 };
 
 static int ili9341_get_modes(struct drm_panel *panel)
@@ -163,7 +170,7 @@ static int ili9341_get_modes(struct drm_panel *panel)
 	struct ili9341 *ili = panel_to_ili9341(panel);
 	struct drm_display_mode *mode;
 
-	mode = drm_mode_duplicate(panel->drm, &prgb_240x320_mode);
+	mode = drm_mode_duplicate(panel->drm, ili->conf->mode);
 	if (!mode) {
 		DRM_DEV_ERROR(panel->drm->dev, "bad mode or failed to add mode\n");
 		return -ENOMEM;
@@ -171,9 +178,6 @@ static int ili9341_get_modes(struct drm_panel *panel)
 
 	drm_mode_set_name(mode);
 
-	mode->width_mm = ili->conf->width_mm;
-	mode->height_mm = ili->conf->height_mm;
-
 	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 |
@@ -184,31 +188,32 @@ static int ili9341_get_modes(struct drm_panel *panel)
 	return 1; /* Number of modes */
 }
 
-static const struct drm_panel_funcs ili9341_drm_funcs = {
+static const struct drm_panel_funcs dt024ctft_drm_funcs = {
 	.disable = ili9341_disable,
 	.unprepare = ili9341_unprepare,
-	.prepare = ili9341_prepare,
+	.prepare = dt024ctft_prepare,
 	.enable = ili9341_enable,
 	.get_modes = ili9341_get_modes,
 };
 
+static const struct ili9341_config dt024ctft_data = {
+	.funcs = &dt024ctft_drm_funcs,
+	.mode = &prgb_240x320_mode,
+};
+
 static int ili9341_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct ili9341 *ili;
-	struct mipi_dbi *mipi;
+	struct mipi_dbi *dbi;
 	struct gpio_desc *dc_gpio;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
-	if (!mipi)
-		return -ENOMEM;
-
 	ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL);
 	if (!ili)
 		return -ENOMEM;
 
-	ili->mipi = mipi;
+	dbi = &ili->dbi;
 
 	spi_set_drvdata(spi, ili);
 
@@ -222,16 +227,16 @@ static int ili9341_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
-	ili->mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(ili->mipi->reset)) {
+	dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(dbi->reset)) {
 		DRM_DEV_ERROR(dev, "failed to get gpio 'reset'\n");
-		return PTR_ERR(ili->mipi->reset);
+		return PTR_ERR(dbi->reset);
 	}
 
-	ili->mipi->backlight = devm_of_find_backlight(dev);
-	if (IS_ERR(ili->mipi->backlight)) {
+	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->mipi->backlight);
+		return PTR_ERR(ili->backlight);
 	}
 
 	dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
@@ -240,7 +245,7 @@ static int ili9341_probe(struct spi_device *spi)
 		return PTR_ERR(dc_gpio);
 	}
 
-	ret = mipi_dbi_spi_init(spi, ili->mipi, dc_gpio);
+	ret = mipi_dbi_spi_init(spi, dbi, dc_gpio);
 	if (ret) {
 		DRM_DEV_ERROR(dev, "MIPI-DBI SPI setup failed\n");
 		return ret;
@@ -248,7 +253,7 @@ static int ili9341_probe(struct spi_device *spi)
 
 	drm_panel_init(&ili->panel);
 	ili->panel.dev = dev;
-	ili->panel.funcs = &ili9341_drm_funcs;
+	ili->panel.funcs = ili->conf->funcs;
 
 	return drm_panel_add(&ili->panel);
 }
@@ -265,11 +270,6 @@ static int ili9341_remove(struct spi_device *spi)
 	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 },
 	{ /* sentinel */ }
-- 
2.20.1

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

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

* [RFC 3/4] drm/mipi-dbi: Support command mode panel drivers
  2019-07-29 19:55 [RFC 0/4] drm/mipi-dbi: Support panel drivers Noralf Trønnes
  2019-07-29 19:55 ` [RFC 1/4] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver Noralf Trønnes
  2019-07-29 19:55 ` [RFC 2/4] drm/panel/ili9341: Rebase and some more Noralf Trønnes
@ 2019-07-29 19:55 ` Noralf Trønnes
  2019-07-29 19:55 ` [RFC 4/4] drm/panel/ili9341: Support mi0283qt Noralf Trønnes
  2019-07-30  6:40 ` [RFC 0/4] drm/mipi-dbi: Support panel drivers Josef Luštický
  4 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-29 19:55 UTC (permalink / raw)
  To: dri-devel; +Cc: daniel.vetter, josef, thierry.reding, laurent.pinchart, sam

This adds a function that registers a DRM driver for use with MIPI DBI
panels in command mode. That is, framebuffer upload over DBI.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_mipi_dbi.c | 110 +++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dbi.h     |   8 +++
 2 files changed, 118 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 1961f713aaab..ef6b843eaf31 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -10,6 +10,7 @@
 #include <linux/dma-buf.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
@@ -17,11 +18,13 @@
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_format_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_mipi_dbi.h>
 #include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_rect.h>
 #include <drm/drm_vblank.h>
@@ -597,6 +600,113 @@ void mipi_dbi_release(struct drm_device *drm)
 }
 EXPORT_SYMBOL(mipi_dbi_release);
 
+static void drm_mipi_dbi_panel_pipe_enable(struct drm_simple_display_pipe *pipe,
+					   struct drm_crtc_state *crtc_state,
+					   struct drm_plane_state *plane_state)
+{
+	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+	struct drm_panel *panel = dbidev->panel;
+	int ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
+
+	DRM_DEBUG_KMS("\n");
+
+	ret = drm_panel_prepare(panel);
+	if (ret)
+		goto out_exit;
+
+	mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+
+	drm_panel_enable(panel);
+out_exit:
+	drm_dev_exit(idx);
+}
+
+static void drm_mipi_dbi_panel_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+	struct drm_panel *panel = dbidev->panel;
+
+	if (!dbidev->enabled)
+		return;
+
+	DRM_DEBUG_KMS("\n");
+
+	dbidev->enabled = false;
+	drm_panel_disable(panel);
+	drm_panel_unprepare(panel);
+}
+
+static const struct drm_simple_display_pipe_funcs drm_mipi_dbi_pipe_funcs = {
+	.enable = drm_mipi_dbi_panel_pipe_enable,
+	.disable = drm_mipi_dbi_panel_pipe_disable,
+	.update = mipi_dbi_pipe_update,
+	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(drm_mipi_dbi_fops);
+
+static struct drm_driver drm_mipi_dbi_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.fops			= &drm_mipi_dbi_fops,
+	.release		= mipi_dbi_release,
+	DRM_GEM_CMA_VMAP_DRIVER_OPS,
+	.debugfs_init		= mipi_dbi_debugfs_init,
+	.name			= "drm_mipi_dbi",
+	.desc			= "DRM MIPI DBI",
+	.date			= "20190729",
+	.major			= 1,
+	.minor			= 0,
+};
+
+/**
+ * drm_mipi_dbi_panel_register - Register a MIPI DBI DRM driver
+ * @panel: DRM Panel
+ * @dbidev: MIPI DBI device structure to initialize
+ * @mode: Display mode
+ *
+ * This function registeres a DRM driver with @panel attached.
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
+				const struct drm_display_mode *mode)
+{
+	struct device *dev = panel->dev;
+	struct drm_device *drm;
+	u32 rotation = 0;
+	int ret;
+
+	dbidev->panel = panel;
+
+	drm = &dbidev->drm;
+	ret = devm_drm_dev_init(dev, drm, &drm_mipi_dbi_driver);
+	if (ret) {
+		kfree(dbidev);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
+	device_property_read_u32(dev, "rotation", &rotation);
+
+	ret = mipi_dbi_dev_init(dbidev, &drm_mipi_dbi_pipe_funcs, mode, rotation);
+
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	drm_fbdev_generic_setup(drm, 16);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_mipi_dbi_panel_register);
+
 /**
  * mipi_dbi_hw_reset - Hardware reset of controller
  * @dbi: MIPI DBI structure
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 67c66f5ee591..e5047208ffb8 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -12,6 +12,7 @@
 #include <drm/drm_device.h>
 #include <drm/drm_simple_kms_helper.h>
 
+struct drm_panel;
 struct drm_rect;
 struct spi_device;
 struct gpio_desc;
@@ -123,6 +124,11 @@ struct mipi_dbi_dev {
 	 * @dbi: MIPI DBI interface
 	 */
 	struct mipi_dbi dbi;
+
+	/**
+	 * @panel: Attached DRM panel. See drm_mipi_dbi_panel_register().
+	 */
+	struct drm_panel *panel;
 };
 
 static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm)
@@ -140,6 +146,8 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
 int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
 		      const struct drm_simple_display_pipe_funcs *funcs,
 		      const struct drm_display_mode *mode, unsigned int rotation);
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
+				const struct drm_display_mode *mode);
 void mipi_dbi_release(struct drm_device *drm);
 void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 			  struct drm_plane_state *old_state);
-- 
2.20.1

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

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

* [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-29 19:55 [RFC 0/4] drm/mipi-dbi: Support panel drivers Noralf Trønnes
                   ` (2 preceding siblings ...)
  2019-07-29 19:55 ` [RFC 3/4] drm/mipi-dbi: Support command mode panel drivers Noralf Trønnes
@ 2019-07-29 19:55 ` Noralf Trønnes
  2019-07-30  8:34   ` Josef Luštický
  2019-07-30 14:30   ` Noralf Trønnes
  2019-07-30  6:40 ` [RFC 0/4] drm/mipi-dbi: Support panel drivers Josef Luštický
  4 siblings, 2 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-29 19:55 UTC (permalink / raw)
  To: dri-devel; +Cc: daniel.vetter, josef, thierry.reding, laurent.pinchart, sam

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
 1 file changed, 170 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index a755f3e60111..dd333a642159 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -21,10 +21,13 @@
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_graph.h>
 #include <linux/spi/spi.h>
 
 #include <video/mipi_display.h>
 
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_modes.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_print.h>
@@ -32,10 +35,25 @@
 
 /* ILI9341 extended register set (Vendor Command Set) */
 #define ILI9341_IFMODE         0xB0 // clock polarity
+#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_IFCTL          0xF6 // interface conrol
 #define ILI9341_PGAMCTRL       0xE0 // positive gamma control
 #define ILI9341_NGAMCTRL       0xE1 // negative gamma control
+#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)
@@ -44,15 +62,18 @@
  * struct ili9341_config - the display specific configuration
  * @funcs: Panel functions
  * @mode: Display mode
+ * @command_mode_only: Panel only supports command mode
  */
 struct ili9341_config {
 	const struct drm_panel_funcs *funcs;
 	const struct drm_display_mode *mode;
+	bool command_mode_only;
 };
 
 struct ili9341 {
 	struct drm_panel panel;
-	struct mipi_dbi dbi;
+	struct mipi_dbi_dev dbidev;
+	bool command_mode;
 	struct backlight_device *backlight;
 	const struct ili9341_config *conf;
 };
@@ -64,7 +85,7 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
 
 static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
 {
-	struct mipi_dbi *dbi = &ili->dbi;
+	struct mipi_dbi *dbi = &ili->dbidev.dbi;
 
 	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
 	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
@@ -74,7 +95,7 @@ static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
 
 static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili)
 {
-	struct mipi_dbi *dbi = &ili->dbi;
+	struct mipi_dbi *dbi = &ili->dbidev.dbi;
 
 	/* HW / SW Reset display and wait */
 	mipi_dbi_hw_reset(dbi);
@@ -164,6 +185,7 @@ static const struct drm_display_mode prgb_240x320_mode = {
 	.height_mm = 49,
 };
 
+/* This is not used in command mode */
 static int ili9341_get_modes(struct drm_panel *panel)
 {
 	struct drm_connector *connector = panel->connector;
@@ -201,19 +223,125 @@ static const struct ili9341_config dt024ctft_data = {
 	.mode = &prgb_240x320_mode,
 };
 
+static int mi0283qt_prepare(struct drm_panel *panel)
+{
+	struct ili9341 *ili = panel_to_ili9341(panel);
+	struct mipi_dbi *dbi = &ili->dbidev.dbi;
+	u8 addr_mode;
+	int ret;
+
+	DRM_DEBUG_KMS("\n");
+
+	ret = mipi_dbi_poweron_conditional_reset(&ili->dbidev);
+	if (ret < 0)
+		return ret;
+	if (ret == 1)
+		goto out_enable;
+	mipi_dbi_hw_reset(dbi);
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+
+	mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30);
+	mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+	mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79);
+	mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+	mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20);
+	mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00);
+
+	/* Power Control */
+	mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x26);
+	mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11);
+	/* VCOM */
+	mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e);
+	mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe);
+
+	/* Memory Access Control */
+	mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
+
+	/* Frame Rate */
+	mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b);
+
+	/* Gamma */
+	mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	mipi_dbi_command(dbi, ILI9341_PGAMCTRL,
+		       0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87,
+		       0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00);
+	mipi_dbi_command(dbi, ILI9341_NGAMCTRL,
+		       0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78,
+		       0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f);
+
+	/* DDRAM */
+	mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07);
+
+	/* Display */
+	mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00);
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(100);
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+	msleep(100);
+
+out_enable:
+	/* The PiTFT (ili9340) has a hardware reset circuit that
+	 * resets only on power-on and not on each reboot through
+	 * a gpio like the rpi-display does.
+	 * As a result, we need to always apply the rotation value
+	 * regardless of the display "on/off" state.
+	 */
+	switch (ili->dbidev.rotation) {
+	default:
+		addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
+			    ILI9341_MADCTL_MX;
+		break;
+	case 90:
+		addr_mode = ILI9341_MADCTL_MY;
+		break;
+	case 180:
+		addr_mode = ILI9341_MADCTL_MV;
+		break;
+	case 270:
+		addr_mode = ILI9341_MADCTL_MX;
+		break;
+	}
+	addr_mode |= ILI9341_MADCTL_BGR;
+	mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+
+	return 0;
+}
+
+static const struct drm_panel_funcs mi0283qt_drm_funcs = {
+	.disable = ili9341_disable,
+	.unprepare = ili9341_unprepare,
+	.prepare = mi0283qt_prepare,
+	.enable = ili9341_enable,
+	.get_modes = ili9341_get_modes,
+};
+
+static const struct drm_display_mode mi0283qt_mode = {
+	DRM_SIMPLE_MODE(320, 240, 58, 43),
+};
+
+static const struct ili9341_config mi0283qt_data = {
+	.funcs = &mi0283qt_drm_funcs,
+	.mode = &mi0283qt_mode,
+	.command_mode_only = true,
+};
+
 static int ili9341_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct ili9341 *ili;
 	struct mipi_dbi *dbi;
 	struct gpio_desc *dc_gpio;
+	struct device_node *port;
 	int ret;
 
-	ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL);
+	ili = kzalloc(sizeof(*ili), GFP_KERNEL);
 	if (!ili)
 		return -ENOMEM;
 
-	dbi = &ili->dbi;
+	dbi = &ili->dbidev.dbi;
 
 	spi_set_drvdata(spi, ili);
 
@@ -255,23 +383,55 @@ static int ili9341_probe(struct spi_device *spi)
 	ili->panel.dev = dev;
 	ili->panel.funcs = ili->conf->funcs;
 
-	return drm_panel_add(&ili->panel);
+	port = of_get_child_by_name(dev->of_node, "port");
+	if (port)
+		of_node_put(port);
+	else
+		ili->command_mode = true;
+
+	if (ili->conf->command_mode_only)
+		ili->command_mode = true;
+
+	if (ili->command_mode)
+		ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, ili->conf->mode);
+	else
+		ret = drm_panel_add(&ili->panel);
+
+	return ret;
 }
 
 static int ili9341_remove(struct spi_device *spi)
 {
 	struct ili9341 *ili = spi_get_drvdata(spi);
 
-	drm_panel_remove(&ili->panel);
+	if (ili->command_mode) {
+		struct drm_device *drm = &ili->dbidev.drm;
 
-	ili9341_disable(&ili->panel);
-	ili9341_unprepare(&ili->panel);
+		drm_dev_unplug(drm);
+		drm_atomic_helper_shutdown(drm);
+	} else {
+		drm_panel_remove(&ili->panel);
+
+		ili9341_disable(&ili->panel);
+		ili9341_unprepare(&ili->panel);
+		kfree(ili);
+	}
 
 	return 0;
 }
 
+static void ili9341_shutdown(struct spi_device *spi)
+{
+	struct ili9341 *ili = spi_get_drvdata(spi);
+
+	if (ili->command_mode)
+		drm_atomic_helper_shutdown(&ili->dbidev.drm);
+}
+
 static const struct of_device_id ili9341_of_match[] = {
 	{ .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data },
+/*	{ .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, */
+	{ .compatible = "mi,mi0283qt", .data = &mi0283qt_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ili9341_of_match);
@@ -279,6 +439,7 @@ MODULE_DEVICE_TABLE(of, ili9341_of_match);
 static struct spi_driver ili9341_driver = {
 	.probe = ili9341_probe,
 	.remove = ili9341_remove,
+	.shutdown = ili9341_shutdown,
 	.driver = {
 		.name = "panel-ilitek-ili9341",
 		.of_match_table = ili9341_of_match,
-- 
2.20.1

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

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

* Re: [RFC 0/4] drm/mipi-dbi: Support panel drivers
  2019-07-29 19:55 [RFC 0/4] drm/mipi-dbi: Support panel drivers Noralf Trønnes
                   ` (3 preceding siblings ...)
  2019-07-29 19:55 ` [RFC 4/4] drm/panel/ili9341: Support mi0283qt Noralf Trønnes
@ 2019-07-30  6:40 ` Josef Luštický
  2019-07-30 14:08   ` Noralf Trønnes
  4 siblings, 1 reply; 19+ messages in thread
From: Josef Luštický @ 2019-07-30  6:40 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: daniel.vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg

Hi Noralf,
the patch series looks good, see comments in the patch emails.

One question: is there a general mechanism to tell a driver not to use
parallel RGB even though
the display supports it and "port" is specified in the device-tree?

Josef

po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>
> Inspired by the thread[1] following the submission of a new ili9341
> panel driver[2], I set out to see if I could support panel drivers in
> drm_mipi_dbi.
>
> I have included the original driver, done some prep work on it, added
> panel support to drm_mipi_dbi and converted mi0283qt to this new panel
> driver.
>
> The big question is whether or not panel drivers should be allowed to
> turn themselves into full fledged DRM drivers.
>
> Noralf.
>
> [1]
> https://lists.freedesktop.org/archives/dri-devel/2019-July/228193.html
> [2] https://patchwork.freedesktop.org/patch/316528/
>
> Josef Lusticky (1):
>   drm/panel: Add Ilitek ILI9341 parallel RGB panel driver
>
> Noralf Trønnes (3):
>   drm/panel/ili9341: Rebase and some more
>   drm/mipi-dbi: Support command mode panel drivers
>   drm/panel/ili9341: Support mi0283qt
>
>  MAINTAINERS                                  |   6 +
>  drivers/gpu/drm/drm_mipi_dbi.c               | 110 +++++
>  drivers/gpu/drm/panel/Kconfig                |   9 +
>  drivers/gpu/drm/panel/Makefile               |   1 +
>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 452 +++++++++++++++++++
>  include/drm/drm_mipi_dbi.h                   |   8 +
>  6 files changed, 586 insertions(+)
>  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

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

* Re: [RFC 2/4] drm/panel/ili9341: Rebase and some more
  2019-07-29 19:55 ` [RFC 2/4] drm/panel/ili9341: Rebase and some more Noralf Trønnes
@ 2019-07-30  6:45   ` Josef Luštický
  0 siblings, 0 replies; 19+ messages in thread
From: Josef Luštický @ 2019-07-30  6:45 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: daniel.vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg

Hi Noralf,
see comment bellow.

po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>
> Embed mipi_dbi in struct ili9341.
> Rebase on moved mipi-dbi, rename variable name.
> Add backlight_device to panel struct.
> mipi_dbi_hw_reset() already has a NULL check on the reset gpio.
> Prepare for more panels by reworking ili9341_config.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 104 +++++++++----------
>  1 file changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> index 0c700b171025..a755f3e60111 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> @@ -28,7 +28,7 @@
>  #include <drm/drm_modes.h>
>  #include <drm/drm_panel.h>
>  #include <drm/drm_print.h>
> -#include <drm/tinydrm/mipi-dbi.h>
> +#include <drm/drm_mipi_dbi.h>
>
>  /* ILI9341 extended register set (Vendor Command Set) */
>  #define ILI9341_IFMODE         0xB0 // clock polarity
> @@ -42,17 +42,18 @@
>
>  /**
>   * struct ili9341_config - the display specific configuration
> - * @width_mm: physical panel width [mm]
> - * @height_mm: physical panel height [mm]
> + * @funcs: Panel functions
> + * @mode: Display mode
>   */
>  struct ili9341_config {
> -       u32 width_mm;
> -       u32 height_mm;
> +       const struct drm_panel_funcs *funcs;
> +       const struct drm_display_mode *mode;
>  };
>
>  struct ili9341 {
>         struct drm_panel panel;
> -       struct mipi_dbi *mipi;
> +       struct mipi_dbi dbi;
> +       struct backlight_device *backlight;
>         const struct ili9341_config *conf;
>  };
>
> @@ -63,47 +64,50 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
>
>  static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
>  {
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_OFF);
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_ENTER_SLEEP_MODE);
> +       struct mipi_dbi *dbi = &ili->dbi;
> +
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> +       mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
>         msleep(5);
>         return 0;
>  }
>
> -static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili)
> +static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili)
I'd prefer to name this function something like ili9341_prgb_init.
In the future, there may be more displays with ILI9341 chip and prgb support
apart from DisplayTech DT024CTFT.
>  {
> -       /* HW / SW Reset display and wait */
> -       if (ili->mipi->reset)
> -               mipi_dbi_hw_reset(ili->mipi);
> +       struct mipi_dbi *dbi = &ili->dbi;
>
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_SOFT_RESET);
> +       /* HW / SW Reset display and wait */
> +       mipi_dbi_hw_reset(dbi);
> +
> +       mipi_dbi_command(dbi, MIPI_DCS_SOFT_RESET);
>         msleep(120);
>
>         /* Polarity */
> -       mipi_dbi_command(ili->mipi, ILI9341_IFMODE, 0xC0);
> +       mipi_dbi_command(dbi, ILI9341_IFMODE, 0xC0);
>
>         /* Interface control */
> -       mipi_dbi_command(ili->mipi, ILI9341_IFCTL, 0x09, 0x01, 0x26);
> +       mipi_dbi_command(dbi, ILI9341_IFCTL, 0x09, 0x01, 0x26);
>
>         /* Pixel format */
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4);
>
>         /* Gamma */
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> -       mipi_dbi_command(ili->mipi, ILI9341_PGAMCTRL,
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> +       mipi_dbi_command(dbi, ILI9341_PGAMCTRL,
>                 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
>                 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> -       mipi_dbi_command(ili->mipi, ILI9341_NGAMCTRL,
> +       mipi_dbi_command(dbi, ILI9341_NGAMCTRL,
>                 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
>                 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
>
>         /* Rotation */
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX);
>
>         /* Exit sleep mode */
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
>         msleep(120);
>
> -       mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_ON);
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
>
>         return 0;
>  }
> @@ -115,12 +119,12 @@ static int ili9341_unprepare(struct drm_panel *panel)
>         return ili9341_deinit(panel, ili);
>  }
>
> -static int ili9341_prepare(struct drm_panel *panel)
> +static int dt024ctft_prepare(struct drm_panel *panel)
>  {
>         struct ili9341 *ili = panel_to_ili9341(panel);
>         int ret;
>
> -       ret = ili9341_init(panel, ili);
> +       ret = dt024ctft_init(panel, ili);
>         if (ret < 0)
>                 ili9341_unprepare(panel);
>         return ret;
> @@ -130,14 +134,14 @@ static int ili9341_enable(struct drm_panel *panel)
>  {
>         struct ili9341 *ili = panel_to_ili9341(panel);
>
> -       return backlight_enable(ili->mipi->backlight);
> +       return backlight_enable(ili->backlight);
>  }
>
>  static int ili9341_disable(struct drm_panel *panel)
>  {
>         struct ili9341 *ili = panel_to_ili9341(panel);
>
> -       return backlight_disable(ili->mipi->backlight);
> +       return backlight_disable(ili->backlight);
>  }
>
>  static const struct drm_display_mode prgb_240x320_mode = {
> @@ -154,7 +158,10 @@ static const struct drm_display_mode prgb_240x320_mode = {
>         .vtotal = 320 + 4 + 2 + 2,
>
>         .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> -       .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED
> +       .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +
> +       .width_mm = 37,
> +       .height_mm = 49,
>  };
>
>  static int ili9341_get_modes(struct drm_panel *panel)
> @@ -163,7 +170,7 @@ static int ili9341_get_modes(struct drm_panel *panel)
>         struct ili9341 *ili = panel_to_ili9341(panel);
>         struct drm_display_mode *mode;
>
> -       mode = drm_mode_duplicate(panel->drm, &prgb_240x320_mode);
> +       mode = drm_mode_duplicate(panel->drm, ili->conf->mode);
>         if (!mode) {
>                 DRM_DEV_ERROR(panel->drm->dev, "bad mode or failed to add mode\n");
>                 return -ENOMEM;
> @@ -171,9 +178,6 @@ static int ili9341_get_modes(struct drm_panel *panel)
>
>         drm_mode_set_name(mode);
>
> -       mode->width_mm = ili->conf->width_mm;
> -       mode->height_mm = ili->conf->height_mm;
> -
>         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 |
> @@ -184,31 +188,32 @@ static int ili9341_get_modes(struct drm_panel *panel)
>         return 1; /* Number of modes */
>  }
>
> -static const struct drm_panel_funcs ili9341_drm_funcs = {
> +static const struct drm_panel_funcs dt024ctft_drm_funcs = {
>         .disable = ili9341_disable,
>         .unprepare = ili9341_unprepare,
> -       .prepare = ili9341_prepare,
> +       .prepare = dt024ctft_prepare,
>         .enable = ili9341_enable,
>         .get_modes = ili9341_get_modes,
>  };
>
> +static const struct ili9341_config dt024ctft_data = {
> +       .funcs = &dt024ctft_drm_funcs,
> +       .mode = &prgb_240x320_mode,
> +};
> +
>  static int ili9341_probe(struct spi_device *spi)
>  {
>         struct device *dev = &spi->dev;
>         struct ili9341 *ili;
> -       struct mipi_dbi *mipi;
> +       struct mipi_dbi *dbi;
>         struct gpio_desc *dc_gpio;
>         int ret;
>
> -       mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> -       if (!mipi)
> -               return -ENOMEM;
> -
>         ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL);
>         if (!ili)
>                 return -ENOMEM;
>
> -       ili->mipi = mipi;
> +       dbi = &ili->dbi;
>
>         spi_set_drvdata(spi, ili);
>
> @@ -222,16 +227,16 @@ static int ili9341_probe(struct spi_device *spi)
>                 return -ENODEV;
>         }
>
> -       ili->mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> -       if (IS_ERR(ili->mipi->reset)) {
> +       dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(dbi->reset)) {
>                 DRM_DEV_ERROR(dev, "failed to get gpio 'reset'\n");
> -               return PTR_ERR(ili->mipi->reset);
> +               return PTR_ERR(dbi->reset);
>         }
>
> -       ili->mipi->backlight = devm_of_find_backlight(dev);
> -       if (IS_ERR(ili->mipi->backlight)) {
> +       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->mipi->backlight);
> +               return PTR_ERR(ili->backlight);
>         }
>
>         dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
> @@ -240,7 +245,7 @@ static int ili9341_probe(struct spi_device *spi)
>                 return PTR_ERR(dc_gpio);
>         }
>
> -       ret = mipi_dbi_spi_init(spi, ili->mipi, dc_gpio);
> +       ret = mipi_dbi_spi_init(spi, dbi, dc_gpio);
>         if (ret) {
>                 DRM_DEV_ERROR(dev, "MIPI-DBI SPI setup failed\n");
>                 return ret;
> @@ -248,7 +253,7 @@ static int ili9341_probe(struct spi_device *spi)
>
>         drm_panel_init(&ili->panel);
>         ili->panel.dev = dev;
> -       ili->panel.funcs = &ili9341_drm_funcs;
> +       ili->panel.funcs = ili->conf->funcs;
>
>         return drm_panel_add(&ili->panel);
>  }
> @@ -265,11 +270,6 @@ static int ili9341_remove(struct spi_device *spi)
>         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 },
>         { /* sentinel */ }
> --
> 2.20.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-29 19:55 ` [RFC 4/4] drm/panel/ili9341: Support mi0283qt Noralf Trønnes
@ 2019-07-30  8:34   ` Josef Luštický
  2019-07-30 14:18     ` Noralf Trønnes
  2019-07-30 14:30   ` Noralf Trønnes
  1 sibling, 1 reply; 19+ messages in thread
From: Josef Luštický @ 2019-07-30  8:34 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: daniel.vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg

Hi Noralf,
see comments bellow.

po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
>  1 file changed, 170 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> index a755f3e60111..dd333a642159 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> @@ -21,10 +21,13 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/of_graph.h>
>  #include <linux/spi/spi.h>
>
>  #include <video/mipi_display.h>
>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_drv.h>
>  #include <drm/drm_modes.h>
>  #include <drm/drm_panel.h>
>  #include <drm/drm_print.h>
> @@ -32,10 +35,25 @@
>
>  /* ILI9341 extended register set (Vendor Command Set) */
>  #define ILI9341_IFMODE         0xB0 // clock polarity
> +#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_IFCTL          0xF6 // interface conrol
>  #define ILI9341_PGAMCTRL       0xE0 // positive gamma control
>  #define ILI9341_NGAMCTRL       0xE1 // negative gamma control
> +#define ILI9341_DTCTRLA        0xe8
> +#define ILI9341_DTCTRLB        0xea
> +#define ILI9341_PWRSEQ         0xed
> +#define ILI9341_EN3GAM         0xf2
> +#define ILI9341_PUMPCTRL       0xf7
Keep the same format for values.
>
> +#define ILI9341_MADCTL_BGR     BIT(3)
>  #define ILI9341_MADCTL_MV      BIT(5)
>  #define ILI9341_MADCTL_MX      BIT(6)
>  #define ILI9341_MADCTL_MY      BIT(7)
> @@ -44,15 +62,18 @@
>   * struct ili9341_config - the display specific configuration
>   * @funcs: Panel functions
>   * @mode: Display mode
> + * @command_mode_only: Panel only supports command mode
Command_mode_only is a bit misleading name - when the MIPI_DBI
interface is used for commands and
image data then there are command and data transfers over the MIPI_DBI
interface.
So "command_mode_only" may confuse users with the usage of
Data/Command pin/bit in MIPI_DBI transfers.
Consider naming this like "serial_mode_only" or "serial_interface_only", or
"prgb_mode_supported" and set to "true" for dt024ctft.

>   */
>  struct ili9341_config {
>         const struct drm_panel_funcs *funcs;
>         const struct drm_display_mode *mode;
> +       bool command_mode_only;
>  };
>
>  struct ili9341 {
>         struct drm_panel panel;
> -       struct mipi_dbi dbi;
> +       struct mipi_dbi_dev dbidev;
> +       bool command_mode;
Similarly to the comment above, this should be named
"serial_input_mode" or (invertably) "prgb_input_mode".

>         struct backlight_device *backlight;
>         const struct ili9341_config *conf;
>  };
> @@ -64,7 +85,7 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
>
>  static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
>  {
> -       struct mipi_dbi *dbi = &ili->dbi;
> +       struct mipi_dbi *dbi = &ili->dbidev.dbi;
>
>         mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
>         mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
> @@ -74,7 +95,7 @@ static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
>
>  static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili)
>  {
> -       struct mipi_dbi *dbi = &ili->dbi;
> +       struct mipi_dbi *dbi = &ili->dbidev.dbi;
>
>         /* HW / SW Reset display and wait */
>         mipi_dbi_hw_reset(dbi);
> @@ -164,6 +185,7 @@ static const struct drm_display_mode prgb_240x320_mode = {
>         .height_mm = 49,
>  };
>
> +/* This is not used in command mode */
>  static int ili9341_get_modes(struct drm_panel *panel)
>  {
>         struct drm_connector *connector = panel->connector;
> @@ -201,19 +223,125 @@ static const struct ili9341_config dt024ctft_data = {
>         .mode = &prgb_240x320_mode,
>  };
>
> +static int mi0283qt_prepare(struct drm_panel *panel)
Consider naming this "ili9341_serial_prepare".

> +{
> +       struct ili9341 *ili = panel_to_ili9341(panel);
> +       struct mipi_dbi *dbi = &ili->dbidev.dbi;
> +       u8 addr_mode;
> +       int ret;
> +
> +       DRM_DEBUG_KMS("\n");
> +
> +       ret = mipi_dbi_poweron_conditional_reset(&ili->dbidev);
> +       if (ret < 0)
> +               return ret;
> +       if (ret == 1)
> +               goto out_enable;
> +       mipi_dbi_hw_reset(dbi);
> +
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> +
> +       mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30);
> +       mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
> +       mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79);
> +       mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
> +       mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20);
> +       mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00);
> +
> +       /* Power Control */
> +       mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x26);
> +       mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11);
> +       /* VCOM */
> +       mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e);
> +       mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe);
> +
> +       /* Memory Access Control */
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
> +
> +       /* Frame Rate */
> +       mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b);
> +
> +       /* Gamma */
> +       mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08);
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> +       mipi_dbi_command(dbi, ILI9341_PGAMCTRL,
> +                      0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87,
> +                      0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00);
> +       mipi_dbi_command(dbi, ILI9341_NGAMCTRL,
> +                      0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78,
> +                      0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f);
> +
> +       /* DDRAM */
> +       mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07);
> +
> +       /* Display */
> +       mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00);
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       msleep(100);
> +
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
> +       msleep(100);
> +
> +out_enable:
> +       /* The PiTFT (ili9340) has a hardware reset circuit that
> +        * resets only on power-on and not on each reboot through
> +        * a gpio like the rpi-display does.
> +        * As a result, we need to always apply the rotation value
> +        * regardless of the display "on/off" state.
> +        */
> +       switch (ili->dbidev.rotation) {
> +       default:
> +               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
> +                           ILI9341_MADCTL_MX;
> +               break;
> +       case 90:
> +               addr_mode = ILI9341_MADCTL_MY;
> +               break;
> +       case 180:
> +               addr_mode = ILI9341_MADCTL_MV;
> +               break;
> +       case 270:
> +               addr_mode = ILI9341_MADCTL_MX;
> +               break;
> +       }
> +       addr_mode |= ILI9341_MADCTL_BGR;
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
> +
> +       return 0;
> +}
> +
> +static const struct drm_panel_funcs mi0283qt_drm_funcs = {
> +       .disable = ili9341_disable,
> +       .unprepare = ili9341_unprepare,
> +       .prepare = mi0283qt_prepare,
> +       .enable = ili9341_enable,
> +       .get_modes = ili9341_get_modes,
> +};
> +
> +static const struct drm_display_mode mi0283qt_mode = {
> +       DRM_SIMPLE_MODE(320, 240, 58, 43),
> +};
> +
> +static const struct ili9341_config mi0283qt_data = {
> +       .funcs = &mi0283qt_drm_funcs,
> +       .mode = &mi0283qt_mode,
> +       .command_mode_only = true,
> +};
> +
>  static int ili9341_probe(struct spi_device *spi)
>  {
>         struct device *dev = &spi->dev;
>         struct ili9341 *ili;
>         struct mipi_dbi *dbi;
>         struct gpio_desc *dc_gpio;
> +       struct device_node *port;
>         int ret;
>
> -       ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL);
> +       ili = kzalloc(sizeof(*ili), GFP_KERNEL);
>         if (!ili)
>                 return -ENOMEM;
>
> -       dbi = &ili->dbi;
> +       dbi = &ili->dbidev.dbi;
>
>         spi_set_drvdata(spi, ili);
>
> @@ -255,23 +383,55 @@ static int ili9341_probe(struct spi_device *spi)
>         ili->panel.dev = dev;
>         ili->panel.funcs = ili->conf->funcs;
>
> -       return drm_panel_add(&ili->panel);
> +       port = of_get_child_by_name(dev->of_node, "port");
> +       if (port)
> +               of_node_put(port);
> +       else
> +               ili->command_mode = true;
> +
> +       if (ili->conf->command_mode_only)
> +               ili->command_mode = true;
> +
> +       if (ili->command_mode)
> +               ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, ili->conf->mode);
> +       else
> +               ret = drm_panel_add(&ili->panel);
> +
> +       return ret;
>  }
>
>  static int ili9341_remove(struct spi_device *spi)
>  {
>         struct ili9341 *ili = spi_get_drvdata(spi);
>
> -       drm_panel_remove(&ili->panel);
> +       if (ili->command_mode) {
> +               struct drm_device *drm = &ili->dbidev.drm;
>
> -       ili9341_disable(&ili->panel);
> -       ili9341_unprepare(&ili->panel);
> +               drm_dev_unplug(drm);
> +               drm_atomic_helper_shutdown(drm);
> +       } else {
> +               drm_panel_remove(&ili->panel);
> +
> +               ili9341_disable(&ili->panel);
> +               ili9341_unprepare(&ili->panel);
> +               kfree(ili);
> +       }
>
>         return 0;
>  }
>
> +static void ili9341_shutdown(struct spi_device *spi)
> +{
> +       struct ili9341 *ili = spi_get_drvdata(spi);
> +
> +       if (ili->command_mode)
> +               drm_atomic_helper_shutdown(&ili->dbidev.drm);
> +}
> +
>  static const struct of_device_id ili9341_of_match[] = {
>         { .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data },
> +/*     { .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, */
> +       { .compatible = "mi,mi0283qt", .data = &mi0283qt_data },
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, ili9341_of_match);
> @@ -279,6 +439,7 @@ MODULE_DEVICE_TABLE(of, ili9341_of_match);
>  static struct spi_driver ili9341_driver = {
>         .probe = ili9341_probe,
>         .remove = ili9341_remove,
> +       .shutdown = ili9341_shutdown,
>         .driver = {
>                 .name = "panel-ilitek-ili9341",
>                 .of_match_table = ili9341_of_match,
> --
> 2.20.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC 0/4] drm/mipi-dbi: Support panel drivers
  2019-07-30  6:40 ` [RFC 0/4] drm/mipi-dbi: Support panel drivers Josef Luštický
@ 2019-07-30 14:08   ` Noralf Trønnes
  2019-07-30 14:27     ` Josef Luštický
  0 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-30 14:08 UTC (permalink / raw)
  To: Josef Luštický
  Cc: daniel.vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg



Den 30.07.2019 08.40, skrev Josef Luštický:
> Hi Noralf,
> the patch series looks good, see comments in the patch emails.
> 
> One question: is there a general mechanism to tell a driver not to use
> parallel RGB even though
> the display supports it and "port" is specified in the device-tree?
> 

Not that I know of. It was just one difference that stood out which
would make it easy to have the same panel driver support both DPI and
DBI pixel mode.

Since your panel has a ili9341 I assmue it has onboard RAM? So you
should be able to drive your display in both modes I guess.

Noralf.

> Josef
> 
> po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>>
>> Inspired by the thread[1] following the submission of a new ili9341
>> panel driver[2], I set out to see if I could support panel drivers in
>> drm_mipi_dbi.
>>
>> I have included the original driver, done some prep work on it, added
>> panel support to drm_mipi_dbi and converted mi0283qt to this new panel
>> driver.
>>
>> The big question is whether or not panel drivers should be allowed to
>> turn themselves into full fledged DRM drivers.
>>
>> Noralf.
>>
>> [1]
>> https://lists.freedesktop.org/archives/dri-devel/2019-July/228193.html
>> [2] https://patchwork.freedesktop.org/patch/316528/
>>
>> Josef Lusticky (1):
>>   drm/panel: Add Ilitek ILI9341 parallel RGB panel driver
>>
>> Noralf Trønnes (3):
>>   drm/panel/ili9341: Rebase and some more
>>   drm/mipi-dbi: Support command mode panel drivers
>>   drm/panel/ili9341: Support mi0283qt
>>
>>  MAINTAINERS                                  |   6 +
>>  drivers/gpu/drm/drm_mipi_dbi.c               | 110 +++++
>>  drivers/gpu/drm/panel/Kconfig                |   9 +
>>  drivers/gpu/drm/panel/Makefile               |   1 +
>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 452 +++++++++++++++++++
>>  include/drm/drm_mipi_dbi.h                   |   8 +
>>  6 files changed, 586 insertions(+)
>>  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

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30  8:34   ` Josef Luštický
@ 2019-07-30 14:18     ` Noralf Trønnes
  0 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-30 14:18 UTC (permalink / raw)
  To: Josef Luštický
  Cc: daniel.vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg



Den 30.07.2019 10.34, skrev Josef Luštický:
> Hi Noralf,
> see comments bellow.
> 
> po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
>>  1 file changed, 170 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
>> index a755f3e60111..dd333a642159 100644
>> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
>> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c

<snip>

>> @@ -44,15 +62,18 @@
>>   * struct ili9341_config - the display specific configuration
>>   * @funcs: Panel functions
>>   * @mode: Display mode
>> + * @command_mode_only: Panel only supports command mode
> Command_mode_only is a bit misleading name - when the MIPI_DBI
> interface is used for commands and
> image data then there are command and data transfers over the MIPI_DBI
> interface.
> So "command_mode_only" may confuse users with the usage of
> Data/Command pin/bit in MIPI_DBI transfers.
> Consider naming this like "serial_mode_only" or "serial_interface_only", or
> "prgb_mode_supported" and set to "true" for dt024ctft.
> 

The reason for choosing this name is that the pixels are uploaded using
a MIPI DCS command. Serial can be confusing since we have MIPI DSI which
is serial, and MIPI DBI can be both serial and parallel.

The MIPI interface name for the rgb pixel interface is DPI.

We could flip the bool and call it dpi_mode.

>>   */
>>  struct ili9341_config {
>>         const struct drm_panel_funcs *funcs;
>>         const struct drm_display_mode *mode;
>> +       bool command_mode_only;
>>  };
>>
>>  struct ili9341 {
>>         struct drm_panel panel;
>> -       struct mipi_dbi dbi;
>> +       struct mipi_dbi_dev dbidev;
>> +       bool command_mode;
> Similarly to the comment above, this should be named
> "serial_input_mode" or (invertably) "prgb_input_mode".
> 
>>         struct backlight_device *backlight;
>>         const struct ili9341_config *conf;
>>  };
>> @@ -64,7 +85,7 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
>>
>>  static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
>>  {
>> -       struct mipi_dbi *dbi = &ili->dbi;
>> +       struct mipi_dbi *dbi = &ili->dbidev.dbi;
>>
>>         mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
>>         mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
>> @@ -74,7 +95,7 @@ static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili)
>>
>>  static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili)
>>  {
>> -       struct mipi_dbi *dbi = &ili->dbi;
>> +       struct mipi_dbi *dbi = &ili->dbidev.dbi;
>>
>>         /* HW / SW Reset display and wait */
>>         mipi_dbi_hw_reset(dbi);
>> @@ -164,6 +185,7 @@ static const struct drm_display_mode prgb_240x320_mode = {
>>         .height_mm = 49,
>>  };
>>
>> +/* This is not used in command mode */
>>  static int ili9341_get_modes(struct drm_panel *panel)
>>  {
>>         struct drm_connector *connector = panel->connector;
>> @@ -201,19 +223,125 @@ static const struct ili9341_config dt024ctft_data = {
>>         .mode = &prgb_240x320_mode,
>>  };
>>
>> +static int mi0283qt_prepare(struct drm_panel *panel)
> Consider naming this "ili9341_serial_prepare".
> 

This is the controller setup function for the MI0283QT panel, hence its
name. It's not a generic controller function.

Noralf.

>> +{
>> +       struct ili9341 *ili = panel_to_ili9341(panel);
>> +       struct mipi_dbi *dbi = &ili->dbidev.dbi;
>> +       u8 addr_mode;
>> +       int ret;
>> +
>> +       DRM_DEBUG_KMS("\n");
>> +
>> +       ret = mipi_dbi_poweron_conditional_reset(&ili->dbidev);
>> +       if (ret < 0)
>> +               return ret;
>> +       if (ret == 1)
>> +               goto out_enable;
>> +       mipi_dbi_hw_reset(dbi);
>> +
>> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
>> +
>> +       mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30);
>> +       mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
>> +       mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79);
>> +       mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
>> +       mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20);
>> +       mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00);
>> +
>> +       /* Power Control */
>> +       mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x26);
>> +       mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11);
>> +       /* VCOM */
>> +       mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e);
>> +       mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe);
>> +
>> +       /* Memory Access Control */
>> +       mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
>> +
>> +       /* Frame Rate */
>> +       mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b);
>> +
>> +       /* Gamma */
>> +       mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08);
>> +       mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
>> +       mipi_dbi_command(dbi, ILI9341_PGAMCTRL,
>> +                      0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87,
>> +                      0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00);
>> +       mipi_dbi_command(dbi, ILI9341_NGAMCTRL,
>> +                      0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78,
>> +                      0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f);
>> +
>> +       /* DDRAM */
>> +       mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07);
>> +
>> +       /* Display */
>> +       mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00);
>> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
>> +       msleep(100);
>> +
>> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
>> +       msleep(100);
>> +
>> +out_enable:
>> +       /* The PiTFT (ili9340) has a hardware reset circuit that
>> +        * resets only on power-on and not on each reboot through
>> +        * a gpio like the rpi-display does.
>> +        * As a result, we need to always apply the rotation value
>> +        * regardless of the display "on/off" state.
>> +        */
>> +       switch (ili->dbidev.rotation) {
>> +       default:
>> +               addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
>> +                           ILI9341_MADCTL_MX;
>> +               break;
>> +       case 90:
>> +               addr_mode = ILI9341_MADCTL_MY;
>> +               break;
>> +       case 180:
>> +               addr_mode = ILI9341_MADCTL_MV;
>> +               break;
>> +       case 270:
>> +               addr_mode = ILI9341_MADCTL_MX;
>> +               break;
>> +       }
>> +       addr_mode |= ILI9341_MADCTL_BGR;
>> +       mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct drm_panel_funcs mi0283qt_drm_funcs = {
>> +       .disable = ili9341_disable,
>> +       .unprepare = ili9341_unprepare,
>> +       .prepare = mi0283qt_prepare,
>> +       .enable = ili9341_enable,
>> +       .get_modes = ili9341_get_modes,
>> +};
>> +
>> +static const struct drm_display_mode mi0283qt_mode = {
>> +       DRM_SIMPLE_MODE(320, 240, 58, 43),
>> +};
>> +
>> +static const struct ili9341_config mi0283qt_data = {
>> +       .funcs = &mi0283qt_drm_funcs,
>> +       .mode = &mi0283qt_mode,
>> +       .command_mode_only = true,
>> +};
>> +
>>  static int ili9341_probe(struct spi_device *spi)
>>  {
>>         struct device *dev = &spi->dev;
>>         struct ili9341 *ili;
>>         struct mipi_dbi *dbi;
>>         struct gpio_desc *dc_gpio;
>> +       struct device_node *port;
>>         int ret;
>>
>> -       ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL);
>> +       ili = kzalloc(sizeof(*ili), GFP_KERNEL);
>>         if (!ili)
>>                 return -ENOMEM;
>>
>> -       dbi = &ili->dbi;
>> +       dbi = &ili->dbidev.dbi;
>>
>>         spi_set_drvdata(spi, ili);
>>
>> @@ -255,23 +383,55 @@ static int ili9341_probe(struct spi_device *spi)
>>         ili->panel.dev = dev;
>>         ili->panel.funcs = ili->conf->funcs;
>>
>> -       return drm_panel_add(&ili->panel);
>> +       port = of_get_child_by_name(dev->of_node, "port");
>> +       if (port)
>> +               of_node_put(port);
>> +       else
>> +               ili->command_mode = true;
>> +
>> +       if (ili->conf->command_mode_only)
>> +               ili->command_mode = true;
>> +
>> +       if (ili->command_mode)
>> +               ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, ili->conf->mode);
>> +       else
>> +               ret = drm_panel_add(&ili->panel);
>> +
>> +       return ret;
>>  }
>>
>>  static int ili9341_remove(struct spi_device *spi)
>>  {
>>         struct ili9341 *ili = spi_get_drvdata(spi);
>>
>> -       drm_panel_remove(&ili->panel);
>> +       if (ili->command_mode) {
>> +               struct drm_device *drm = &ili->dbidev.drm;
>>
>> -       ili9341_disable(&ili->panel);
>> -       ili9341_unprepare(&ili->panel);
>> +               drm_dev_unplug(drm);
>> +               drm_atomic_helper_shutdown(drm);
>> +       } else {
>> +               drm_panel_remove(&ili->panel);
>> +
>> +               ili9341_disable(&ili->panel);
>> +               ili9341_unprepare(&ili->panel);
>> +               kfree(ili);
>> +       }
>>
>>         return 0;
>>  }
>>
>> +static void ili9341_shutdown(struct spi_device *spi)
>> +{
>> +       struct ili9341 *ili = spi_get_drvdata(spi);
>> +
>> +       if (ili->command_mode)
>> +               drm_atomic_helper_shutdown(&ili->dbidev.drm);
>> +}
>> +
>>  static const struct of_device_id ili9341_of_match[] = {
>>         { .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data },
>> +/*     { .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, */
>> +       { .compatible = "mi,mi0283qt", .data = &mi0283qt_data },
>>         { /* sentinel */ }
>>  };
>>  MODULE_DEVICE_TABLE(of, ili9341_of_match);
>> @@ -279,6 +439,7 @@ MODULE_DEVICE_TABLE(of, ili9341_of_match);
>>  static struct spi_driver ili9341_driver = {
>>         .probe = ili9341_probe,
>>         .remove = ili9341_remove,
>> +       .shutdown = ili9341_shutdown,
>>         .driver = {
>>                 .name = "panel-ilitek-ili9341",
>>                 .of_match_table = ili9341_of_match,
>> --
>> 2.20.1
>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC 0/4] drm/mipi-dbi: Support panel drivers
  2019-07-30 14:08   ` Noralf Trønnes
@ 2019-07-30 14:27     ` Josef Luštický
  2019-07-30 14:51       ` Noralf Trønnes
  0 siblings, 1 reply; 19+ messages in thread
From: Josef Luštický @ 2019-07-30 14:27 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: Daniel Vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg

Yes, the onboard RAM is ili9341 feature.
I am able to drive the DisplayTech DT024CTFT panel in both modes.

I've just tested the DisplayTech DT024CTFT panel with your patchset
and it works fine with parallel RGB input mode:
compatible = "displaytech,dt024ctft", "ilitek,ili9341";
and with SPI input mode:
compatible = "mi,mi0283qt", "ilitek,ili9341";

However, there seems to be an issue when i use
compatible = "displaytech,dt024ctft", "ilitek,ili9341";
but no port property.
This is caused by calling dt024ctft_prepare in such case.

I would expected the driver to fallback to SPI input mode in such case.
In other words, the ili9341-based panels can be used in both modes,
but not all displays available
on the market provide breakout pins for the parallel RGB connection.

út 30. 7. 2019 v 16:08 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>
>
>
> Den 30.07.2019 08.40, skrev Josef Luštický:
> > Hi Noralf,
> > the patch series looks good, see comments in the patch emails.
> >
> > One question: is there a general mechanism to tell a driver not to use
> > parallel RGB even though
> > the display supports it and "port" is specified in the device-tree?
> >
>
> Not that I know of. It was just one difference that stood out which
> would make it easy to have the same panel driver support both DPI and
> DBI pixel mode.
>
> Since your panel has a ili9341 I assmue it has onboard RAM? So you
> should be able to drive your display in both modes I guess.
>
> Noralf.
>
> > Josef
> >
> > po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
> >>
> >> Inspired by the thread[1] following the submission of a new ili9341
> >> panel driver[2], I set out to see if I could support panel drivers in
> >> drm_mipi_dbi.
> >>
> >> I have included the original driver, done some prep work on it, added
> >> panel support to drm_mipi_dbi and converted mi0283qt to this new panel
> >> driver.
> >>
> >> The big question is whether or not panel drivers should be allowed to
> >> turn themselves into full fledged DRM drivers.
> >>
> >> Noralf.
> >>
> >> [1]
> >> https://lists.freedesktop.org/archives/dri-devel/2019-July/228193.html
> >> [2] https://patchwork.freedesktop.org/patch/316528/
> >>
> >> Josef Lusticky (1):
> >>   drm/panel: Add Ilitek ILI9341 parallel RGB panel driver
> >>
> >> Noralf Trønnes (3):
> >>   drm/panel/ili9341: Rebase and some more
> >>   drm/mipi-dbi: Support command mode panel drivers
> >>   drm/panel/ili9341: Support mi0283qt
> >>
> >>  MAINTAINERS                                  |   6 +
> >>  drivers/gpu/drm/drm_mipi_dbi.c               | 110 +++++
> >>  drivers/gpu/drm/panel/Kconfig                |   9 +
> >>  drivers/gpu/drm/panel/Makefile               |   1 +
> >>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 452 +++++++++++++++++++
> >>  include/drm/drm_mipi_dbi.h                   |   8 +
> >>  6 files changed, 586 insertions(+)
> >>  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

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-29 19:55 ` [RFC 4/4] drm/panel/ili9341: Support mi0283qt Noralf Trønnes
  2019-07-30  8:34   ` Josef Luštický
@ 2019-07-30 14:30   ` Noralf Trønnes
  2019-07-30 15:22     ` Daniel Vetter
  1 sibling, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-30 14:30 UTC (permalink / raw)
  To: dri-devel; +Cc: daniel.vetter, josef, thierry.reding, laurent.pinchart, sam



Den 29.07.2019 21.55, skrev Noralf Trønnes:
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
>  1 file changed, 170 insertions(+), 9 deletions(-)
> 

I have realised that this will change the DRM driver name from mi0283qt
to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
support. I haven't tried it, but this is the first thing that gives this
driver any real advantage over its fbdev counterpart in
drivers/staging/fbtft, so I don't want to loose that.
So even if MIPI DBI panel support is implemented in some form, I will
wait with converting mi0283qt until someone has updated the kmsro driver.

Noralf.

[1]
https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/winsys/kmsro/drm
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/targets/dri/target.c
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC 0/4] drm/mipi-dbi: Support panel drivers
  2019-07-30 14:27     ` Josef Luštický
@ 2019-07-30 14:51       ` Noralf Trønnes
  0 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-30 14:51 UTC (permalink / raw)
  To: Josef Luštický
  Cc: Daniel Vetter, dri-devel, Thierry Reding, Laurent Pinchart, Sam Ravnborg

Please reply inline instead of top-posting.

Den 30.07.2019 16.27, skrev Josef Luštický:
> Yes, the onboard RAM is ili9341 feature.
> I am able to drive the DisplayTech DT024CTFT panel in both modes.
> 
> I've just tested the DisplayTech DT024CTFT panel with your patchset
> and it works fine with parallel RGB input mode:
> compatible = "displaytech,dt024ctft", "ilitek,ili9341";
> and with SPI input mode:
> compatible = "mi,mi0283qt", "ilitek,ili9341";
> 
> However, there seems to be an issue when i use
> compatible = "displaytech,dt024ctft", "ilitek,ili9341";
> but no port property.
> This is caused by calling dt024ctft_prepare in such case.
> 
> I would expected the driver to fallback to SPI input mode in such case.
> In other words, the ili9341-based panels can be used in both modes,
> but not all displays available
> on the market provide breakout pins for the parallel RGB connection.
> 

You need to support both modes in dt024ctft_prepare and apply the
appropriate controller setup for the chosen pixel interface. The setup
is panel specific and interface specific.

Unless your panel is specced the exact same way as the mi0283qt panel,
then the dt024ctft panel will have different power pump setup, gamma
curve etc. So each panel needs its own prepare function. Some panels
even have commands in their setup that isn't listed in the controller
datasheet, ie. they have a custom controller for their display. Some
even pretend to be one controller, but reading the register contents,
it's doubtful that this is true and it's more likely to be a clone.

Noralf.

> út 30. 7. 2019 v 16:08 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>>
>>
>>
>> Den 30.07.2019 08.40, skrev Josef Luštický:
>>> Hi Noralf,
>>> the patch series looks good, see comments in the patch emails.
>>>
>>> One question: is there a general mechanism to tell a driver not to use
>>> parallel RGB even though
>>> the display supports it and "port" is specified in the device-tree?
>>>
>>
>> Not that I know of. It was just one difference that stood out which
>> would make it easy to have the same panel driver support both DPI and
>> DBI pixel mode.
>>
>> Since your panel has a ili9341 I assmue it has onboard RAM? So you
>> should be able to drive your display in both modes I guess.
>>
>> Noralf.
>>
>>> Josef
>>>
>>> po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal:
>>>>
>>>> Inspired by the thread[1] following the submission of a new ili9341
>>>> panel driver[2], I set out to see if I could support panel drivers in
>>>> drm_mipi_dbi.
>>>>
>>>> I have included the original driver, done some prep work on it, added
>>>> panel support to drm_mipi_dbi and converted mi0283qt to this new panel
>>>> driver.
>>>>
>>>> The big question is whether or not panel drivers should be allowed to
>>>> turn themselves into full fledged DRM drivers.
>>>>
>>>> Noralf.
>>>>
>>>> [1]
>>>> https://lists.freedesktop.org/archives/dri-devel/2019-July/228193.html
>>>> [2] https://patchwork.freedesktop.org/patch/316528/
>>>>
>>>> Josef Lusticky (1):
>>>>   drm/panel: Add Ilitek ILI9341 parallel RGB panel driver
>>>>
>>>> Noralf Trønnes (3):
>>>>   drm/panel/ili9341: Rebase and some more
>>>>   drm/mipi-dbi: Support command mode panel drivers
>>>>   drm/panel/ili9341: Support mi0283qt
>>>>
>>>>  MAINTAINERS                                  |   6 +
>>>>  drivers/gpu/drm/drm_mipi_dbi.c               | 110 +++++
>>>>  drivers/gpu/drm/panel/Kconfig                |   9 +
>>>>  drivers/gpu/drm/panel/Makefile               |   1 +
>>>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 452 +++++++++++++++++++
>>>>  include/drm/drm_mipi_dbi.h                   |   8 +
>>>>  6 files changed, 586 insertions(+)
>>>>  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

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30 14:30   ` Noralf Trønnes
@ 2019-07-30 15:22     ` Daniel Vetter
  2019-07-30 17:04       ` Noralf Trønnes
  2019-07-30 17:12       ` Emil Velikov
  0 siblings, 2 replies; 19+ messages in thread
From: Daniel Vetter @ 2019-07-30 15:22 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: Josef Lusticky, dri-devel, Thierry Reding, Laurent Pinchart,
	Sam Ravnborg

On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>
>
> Den 29.07.2019 21.55, skrev Noralf Trønnes:
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > ---
> >  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
> >  1 file changed, 170 insertions(+), 9 deletions(-)
> >
>
> I have realised that this will change the DRM driver name from mi0283qt
> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
> support. I haven't tried it, but this is the first thing that gives this
> driver any real advantage over its fbdev counterpart in
> drivers/staging/fbtft, so I don't want to loose that.
> So even if MIPI DBI panel support is implemented in some form, I will
> wait with converting mi0283qt until someone has updated the kmsro driver.

Why does it change? You should be able to stuff whatever you feel like
into the drm driver name, this doesn't have to match either your
platform/spi/whatever driver name nor the module option.
-Daniel


> Noralf.
>
> [1]
> https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/winsys/kmsro/drm
> https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/targets/dri/target.c



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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] 19+ messages in thread

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30 15:22     ` Daniel Vetter
@ 2019-07-30 17:04       ` Noralf Trønnes
  2019-07-30 17:12       ` Emil Velikov
  1 sibling, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-30 17:04 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Josef Lusticky, dri-devel, Thierry Reding, Laurent Pinchart,
	Sam Ravnborg



Den 30.07.2019 17.22, skrev Daniel Vetter:
> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>>
>>
>> Den 29.07.2019 21.55, skrev Noralf Trønnes:
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
>>>  1 file changed, 170 insertions(+), 9 deletions(-)
>>>
>>
>> I have realised that this will change the DRM driver name from mi0283qt
>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
>> support. I haven't tried it, but this is the first thing that gives this
>> driver any real advantage over its fbdev counterpart in
>> drivers/staging/fbtft, so I don't want to loose that.
>> So even if MIPI DBI panel support is implemented in some form, I will
>> wait with converting mi0283qt until someone has updated the kmsro driver.
> 
> Why does it change? You should be able to stuff whatever you feel like
> into the drm driver name, this doesn't have to match either your
> platform/spi/whatever driver name nor the module option.

Strictly it doesn't have to change, I could pass a mi0283qt specific
struct drm_driver to drm_mipi_dbi_panel_register() and keep the DRM
driver name.

Noralf.

> -Daniel
> 
> 
>> Noralf.
>>
>> [1]
>> https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/winsys/kmsro/drm
>> https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/targets/dri/target.c
> 
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30 15:22     ` Daniel Vetter
  2019-07-30 17:04       ` Noralf Trønnes
@ 2019-07-30 17:12       ` Emil Velikov
  2019-07-30 17:33         ` Noralf Trønnes
  1 sibling, 1 reply; 19+ messages in thread
From: Emil Velikov @ 2019-07-30 17:12 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Josef Lusticky, dri-devel, Thierry Reding, Laurent Pinchart,
	Sam Ravnborg

On 2019/07/30, Daniel Vetter wrote:
> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> >
> >
> >
> > Den 29.07.2019 21.55, skrev Noralf Trønnes:
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > ---
> > >  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
> > >  1 file changed, 170 insertions(+), 9 deletions(-)
> > >
> >
> > I have realised that this will change the DRM driver name from mi0283qt
> > to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
> > support. I haven't tried it, but this is the first thing that gives this
> > driver any real advantage over its fbdev counterpart in
> > drivers/staging/fbtft, so I don't want to loose that.
> > So even if MIPI DBI panel support is implemented in some form, I will
> > wait with converting mi0283qt until someone has updated the kmsro driver.
> 
> Why does it change? You should be able to stuff whatever you feel like
> into the drm driver name, this doesn't have to match either your
> platform/spi/whatever driver name nor the module option.

Last time i've looked DRM drivers using the mipi dsi helpers do _not_
have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi
dbi should not have "drm_mipi_dbi".

That said, we should probably highlight even more that the driver name
is an ABI.

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

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30 17:12       ` Emil Velikov
@ 2019-07-30 17:33         ` Noralf Trønnes
  2019-07-31  8:23           ` Daniel Vetter
  2019-07-31 13:35           ` Emil Velikov
  0 siblings, 2 replies; 19+ messages in thread
From: Noralf Trønnes @ 2019-07-30 17:33 UTC (permalink / raw)
  To: Emil Velikov, Daniel Vetter
  Cc: Thierry Reding, Sam Ravnborg, Josef Lusticky, dri-devel,
	Laurent Pinchart



Den 30.07.2019 19.12, skrev Emil Velikov:
> On 2019/07/30, Daniel Vetter wrote:
>> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>>>
>>>
>>>
>>> Den 29.07.2019 21.55, skrev Noralf Trønnes:
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
>>>>  1 file changed, 170 insertions(+), 9 deletions(-)
>>>>
>>>
>>> I have realised that this will change the DRM driver name from mi0283qt
>>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
>>> support. I haven't tried it, but this is the first thing that gives this
>>> driver any real advantage over its fbdev counterpart in
>>> drivers/staging/fbtft, so I don't want to loose that.
>>> So even if MIPI DBI panel support is implemented in some form, I will
>>> wait with converting mi0283qt until someone has updated the kmsro driver.
>>
>> Why does it change? You should be able to stuff whatever you feel like
>> into the drm driver name, this doesn't have to match either your
>> platform/spi/whatever driver name nor the module option.
> 
> Last time i've looked DRM drivers using the mipi dsi helpers do _not_
> have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi
> dbi should not have "drm_mipi_dbi".
> 

What purpose does the DRM driver name serve for userspace?
Why can't it be called drm_mipi_dbi? Because multiple panel drivers will
use the same name? You're statement implies that there are some rules
regarding DRM driver naming.

> That said, we should probably highlight even more that the driver name
> is an ABI.
> 

This I didn't know.

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

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

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30 17:33         ` Noralf Trønnes
@ 2019-07-31  8:23           ` Daniel Vetter
  2019-07-31 13:35           ` Emil Velikov
  1 sibling, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2019-07-31  8:23 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: Daniel Vetter, Emil Velikov, Josef Lusticky, dri-devel,
	Thierry Reding, Laurent Pinchart, Sam Ravnborg

On Tue, Jul 30, 2019 at 07:33:28PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 30.07.2019 19.12, skrev Emil Velikov:
> > On 2019/07/30, Daniel Vetter wrote:
> >> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> >>>
> >>>
> >>>
> >>> Den 29.07.2019 21.55, skrev Noralf Trønnes:
> >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >>>> ---
> >>>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
> >>>>  1 file changed, 170 insertions(+), 9 deletions(-)
> >>>>
> >>>
> >>> I have realised that this will change the DRM driver name from mi0283qt
> >>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
> >>> support. I haven't tried it, but this is the first thing that gives this
> >>> driver any real advantage over its fbdev counterpart in
> >>> drivers/staging/fbtft, so I don't want to loose that.
> >>> So even if MIPI DBI panel support is implemented in some form, I will
> >>> wait with converting mi0283qt until someone has updated the kmsro driver.
> >>
> >> Why does it change? You should be able to stuff whatever you feel like
> >> into the drm driver name, this doesn't have to match either your
> >> platform/spi/whatever driver name nor the module option.
> > 
> > Last time i've looked DRM drivers using the mipi dsi helpers do _not_
> > have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi
> > dbi should not have "drm_mipi_dbi".
> > 
> 
> What purpose does the DRM driver name serve for userspace?
> Why can't it be called drm_mipi_dbi? Because multiple panel drivers will
> use the same name? You're statement implies that there are some rules
> regarding DRM driver naming.

Worst case kmalloc a drm_driver at runtime and set the driver name to
match the panel name? Imo that makes sense for these
panel-as-full-drm_driver drivers ...

> > That said, we should probably highlight even more that the driver name
> > is an ABI.
> > 
> 
> This I didn't know.

kmsro and mesa in general uses it to figure out which userspace driver
needs to be loaded for which kernel driver. That makes it uapi, and yeah I
guess we should document it. I think aside from kmsro it's not relevant
for display-only drivers, but for anything that does rendering and has
custom ioctls it very much is the only real way to figure out what kind of
driver you have.
-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] 19+ messages in thread

* Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt
  2019-07-30 17:33         ` Noralf Trønnes
  2019-07-31  8:23           ` Daniel Vetter
@ 2019-07-31 13:35           ` Emil Velikov
  1 sibling, 0 replies; 19+ messages in thread
From: Emil Velikov @ 2019-07-31 13:35 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: Daniel Vetter, Josef Lusticky, dri-devel, Thierry Reding,
	Laurent Pinchart, Sam Ravnborg

On Tue, 30 Jul 2019 at 18:33, Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 30.07.2019 19.12, skrev Emil Velikov:
> > On 2019/07/30, Daniel Vetter wrote:
> >> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> >>>
> >>>
> >>>
> >>> Den 29.07.2019 21.55, skrev Noralf Trønnes:
> >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >>>> ---
> >>>>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++-
> >>>>  1 file changed, 170 insertions(+), 9 deletions(-)
> >>>>
> >>>
> >>> I have realised that this will change the DRM driver name from mi0283qt
> >>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1]
> >>> support. I haven't tried it, but this is the first thing that gives this
> >>> driver any real advantage over its fbdev counterpart in
> >>> drivers/staging/fbtft, so I don't want to loose that.
> >>> So even if MIPI DBI panel support is implemented in some form, I will
> >>> wait with converting mi0283qt until someone has updated the kmsro driver.
> >>
> >> Why does it change? You should be able to stuff whatever you feel like
> >> into the drm driver name, this doesn't have to match either your
> >> platform/spi/whatever driver name nor the module option.
> >
> > Last time i've looked DRM drivers using the mipi dsi helpers do _not_
> > have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi
> > dbi should not have "drm_mipi_dbi".
> >
>
> What purpose does the DRM driver name serve for userspace?
> Why can't it be called drm_mipi_dbi? Because multiple panel drivers will
> use the same name? You're statement implies that there are some rules
> regarding DRM driver naming.
>
As you point out in the kmsro case - we use those to "pair" KMS with GPU device.
We could use another, more robust solution, yet ETIME so this will do for now.

> > That said, we should probably highlight even more that the driver name
> > is an ABI.
> >
>
> This I didn't know.
>
One simple example that has existed for years is amdgpu vs radeon distinction.
In Mesa the radeonsi driver could use either driver. Although
interacting with each one happens via a different set of ioctls.
Additionally, a different set of features (and workarounds) is used
depending on the version exposed by the driver.

All these fields (and a bit more) are exposed to userspace via the
drm_version() function/ioctl.

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

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

end of thread, other threads:[~2019-07-31 13:35 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-29 19:55 [RFC 0/4] drm/mipi-dbi: Support panel drivers Noralf Trønnes
2019-07-29 19:55 ` [RFC 1/4] drm/panel: Add Ilitek ILI9341 parallel RGB panel driver Noralf Trønnes
2019-07-29 19:55 ` [RFC 2/4] drm/panel/ili9341: Rebase and some more Noralf Trønnes
2019-07-30  6:45   ` Josef Luštický
2019-07-29 19:55 ` [RFC 3/4] drm/mipi-dbi: Support command mode panel drivers Noralf Trønnes
2019-07-29 19:55 ` [RFC 4/4] drm/panel/ili9341: Support mi0283qt Noralf Trønnes
2019-07-30  8:34   ` Josef Luštický
2019-07-30 14:18     ` Noralf Trønnes
2019-07-30 14:30   ` Noralf Trønnes
2019-07-30 15:22     ` Daniel Vetter
2019-07-30 17:04       ` Noralf Trønnes
2019-07-30 17:12       ` Emil Velikov
2019-07-30 17:33         ` Noralf Trønnes
2019-07-31  8:23           ` Daniel Vetter
2019-07-31 13:35           ` Emil Velikov
2019-07-30  6:40 ` [RFC 0/4] drm/mipi-dbi: Support panel drivers Josef Luštický
2019-07-30 14:08   ` Noralf Trønnes
2019-07-30 14:27     ` Josef Luštický
2019-07-30 14:51       ` 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.