[RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Paul Cercueil]

Hi,

Here's a follow-up on the previous discussion about the current state of
DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.

This patchset introduces the following:
* It slightly tweaks the MIPI DSI code so that it supports MIPI DBI over
  various buses. This patch has been tested with a non-upstream DRM
  panel driver for a ILI9331 DBI/8080 panel, written with the DSI
  framework (and doesn't include <drm/drm_mipi_dbi.h>), and non-upstream
  DSI/DBI host driver for the Ingenic SoCs.

* A SPI DBI host driver, using the current MIPI DSI framework. It allows
  MIPI DSI/DBI drivers to be written with the DSI framework, even if
  they are connected over SPI, instead of registering as SPI device
  drivers. Since most of these panels can be connected over various
  buses, it permits to reuse the same driver independently of the bus
  used.

* A TinyDRM driver for DSI/DBI panels, once again independent of the bus
  used; the only dependency (currently) being that the panel must
  understand DCS commands.

* A DRM panel driver to test the stack. This driver controls Ilitek
  ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
  TFT LCD panel. This panel was converted from
  drivers/gpu/drm/tiny/ili9341.c.

I would like to emphasize that while it has been compile-tested, I did
not test it with real hardware since I do not have any DBI panel
connected over SPI. I did runtime-test the code, just without any panel
connected.

Another thing to note, is that it does not break Device Tree ABI. The
display node stays the same:

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

The reason it works, is that the "adafruit,yx240qv29" device is probed
on the SPI bus, so it will match with the SPI/DBI host driver. This will
in turn register the very same node with the DSI bus, and the ILI9341
DRM panel driver will probe. The driver will detect that no controller
is linked to the panel, and eventually register the DBI/DSI TinyDRM
driver.

I can't stress it enough that this is a RFC, so it still has very rough
edges.

Cheers,
-Paul

Paul Cercueil (4):
  gpu/drm: dsi: Let host and device specify supported bus
  gpu/drm: Add SPI DBI host driver
  gpu/drm: Add TinyDRM for DSI/DBI panels
  gpu/drm: Add Ilitek ILI9341 DBI panel driver

 drivers/gpu/drm/bridge/Kconfig               |   8 +
 drivers/gpu/drm/bridge/Makefile              |   1 +
 drivers/gpu/drm/bridge/dbi-spi.c             | 159 +++++++++
 drivers/gpu/drm/drm_mipi_dsi.c               |   9 +
 drivers/gpu/drm/panel/Kconfig                |   9 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 347 +++++++++++++++++++
 drivers/gpu/drm/tiny/Kconfig                 |   8 +
 drivers/gpu/drm/tiny/Makefile                |   1 +
 drivers/gpu/drm/tiny/tiny-dsi.c              | 262 ++++++++++++++
 include/drm/drm_mipi_dsi.h                   |  31 ++
 11 files changed, 836 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/dbi-spi.c
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c
 create mode 100644 drivers/gpu/drm/tiny/tiny-dsi.c

-- 
2.26.2

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

[RFC PATCH 1/4] gpu/drm: dsi: Let host and device specify supported bus

[Paul Cercueil]

The current MIPI DSI framework can very well be used to support MIPI DBI
panels. In order to add support for the various bus types supported by
DBI, the DRM panel drivers should specify the bus type they will use,
and the DSI host drivers should specify the bus types they are
compatible with.

The DSI host driver can then use the information provided by the DBI/DSI
device driver, such as the bus type and the number of lanes, to
configure its hardware properly.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_mipi_dsi.c |  9 +++++++++
 include/drm/drm_mipi_dsi.h     | 12 ++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 55531895dde6..fe0d874d1594 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -282,6 +282,9 @@ int mipi_dsi_host_register(struct mipi_dsi_host *host)
 {
 	struct device_node *node;
 
+	if (WARN_ON_ONCE(!host->bus_types))
+		host->bus_types = MIPI_DEVICE_TYPE_DSI;
+
 	for_each_available_child_of_node(host->dev->of_node, node) {
 		/* skip nodes without reg property */
 		if (!of_find_property(node, "reg", NULL))
@@ -324,6 +327,12 @@ int mipi_dsi_attach(struct mipi_dsi_device *dsi)
 {
 	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
 
+	if (WARN_ON_ONCE(!dsi->bus_type))
+		dsi->bus_type = MIPI_DEVICE_TYPE_DSI;
+
+	if (!(dsi->bus_type & dsi->host->bus_types))
+		return -ENOTSUPP;
+
 	if (!ops || !ops->attach)
 		return -ENOSYS;
 
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 360e6377e84b..65d2961fc054 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -63,6 +63,14 @@ struct mipi_dsi_packet {
 int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
 			   const struct mipi_dsi_msg *msg);
 
+/* MIPI bus types */
+#define MIPI_DEVICE_TYPE_DSI		BIT(0)
+#define MIPI_DEVICE_TYPE_DBI_SPI_MODE1	BIT(1)
+#define MIPI_DEVICE_TYPE_DBI_SPI_MODE2	BIT(2)
+#define MIPI_DEVICE_TYPE_DBI_SPI_MODE3	BIT(3)
+#define MIPI_DEVICE_TYPE_DBI_M6800	BIT(4)
+#define MIPI_DEVICE_TYPE_DBI_I8080	BIT(5)
+
 /**
  * struct mipi_dsi_host_ops - DSI bus operations
  * @attach: attach DSI device to DSI host
@@ -94,11 +102,13 @@ struct mipi_dsi_host_ops {
  * struct mipi_dsi_host - DSI host device
  * @dev: driver model device node for this DSI host
  * @ops: DSI host operations
+ * @bus_types: Bitmask of supported MIPI bus types
  * @list: list management
  */
 struct mipi_dsi_host {
 	struct device *dev;
 	const struct mipi_dsi_host_ops *ops;
+	unsigned int bus_types;
 	struct list_head list;
 };
 
@@ -162,6 +172,7 @@ struct mipi_dsi_device_info {
  * @host: DSI host for this peripheral
  * @dev: driver model device node for this peripheral
  * @name: DSI peripheral chip type
+ * @bus_type: MIPI bus type (MIPI_DEVICE_TYPE_DSI/...)
  * @channel: virtual channel assigned to the peripheral
  * @format: pixel format for video mode
  * @lanes: number of active data lanes
@@ -178,6 +189,7 @@ struct mipi_dsi_device {
 	struct device dev;
 
 	char name[DSI_DEV_NAME_SIZE];
+	unsigned int bus_type;
 	unsigned int channel;
 	unsigned int lanes;
 	enum mipi_dsi_pixel_format format;
-- 
2.26.2

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

[RFC PATCH 2/4] gpu/drm: Add SPI DBI host driver

[Paul Cercueil]

This driver will register a DBI host driver for panels connected over
SPI.

For now, only DBI type c3 is supported, which is a SPI protocol with 8
bits per word, with the data/command information carried by a separate
GPIO.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/bridge/Kconfig   |   8 ++
 drivers/gpu/drm/bridge/Makefile  |   1 +
 drivers/gpu/drm/bridge/dbi-spi.c | 159 +++++++++++++++++++++++++++++++
 3 files changed, 168 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/dbi-spi.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index aaed2347ace9..61587bc4d6e6 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -180,6 +180,14 @@ config DRM_TI_TPD12S015
 	  Texas Instruments TPD12S015 HDMI level shifter and ESD protection
 	  driver.
 
+config DRM_MIPI_DBI_SPI
+	tristate "SPI host support for MIPI DBI"
+	depends on OF && SPI
+	select DRM_MIPI_DSI
+	select DRM_MIPI_DBI
+	help
+	  Driver to support DBI over SPI.
+
 source "drivers/gpu/drm/bridge/analogix/Kconfig"
 
 source "drivers/gpu/drm/bridge/adv7511/Kconfig"
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 6fb062b5b0f0..17fdf7de498d 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
 obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o
+obj-$(CONFIG_DRM_MIPI_DBI_SPI) += dbi-spi.o
 
 obj-y += analogix/
 obj-y += synopsys/
diff --git a/drivers/gpu/drm/bridge/dbi-spi.c b/drivers/gpu/drm/bridge/dbi-spi.c
new file mode 100644
index 000000000000..c790514b7372
--- /dev/null
+++ b/drivers/gpu/drm/bridge/dbi-spi.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MIPI Display Bus Interface (DBI) SPI support
+ *
+ * Copyright 2016 Noralf Trønnes
+ * Copyright 2020 Paul Cercueil <paul@crapouillou.net>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+
+#include <video/mipi_display.h>
+
+struct dbi_spi {
+	struct mipi_dsi_host host;
+	struct mipi_dsi_host_ops host_ops;
+
+	struct spi_device *spi;
+	struct gpio_desc *dc;
+	struct mutex cmdlock;
+};
+
+static inline struct dbi_spi *host_to_dbi_spi(struct mipi_dsi_host *host)
+{
+	return container_of(host, struct dbi_spi, host);
+}
+
+static ssize_t dbi_spi_transfer(struct mipi_dsi_host *host,
+				const struct mipi_dsi_msg *msg)
+{
+	struct dbi_spi *dbi = host_to_dbi_spi(host);
+	struct spi_device *spi = dbi->spi;
+	const u8 *buf = msg->tx_buf;
+	unsigned int bpw = 8;
+	u32 speed_hz;
+	ssize_t ret;
+
+	/* for now we only support sending messages, not receiving */
+	if (msg->rx_len)
+		return -ENOTSUPP;
+
+	gpiod_set_value_cansleep(dbi->dc, 0);
+
+	speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
+	ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, buf, 1);
+	if (ret || msg->tx_len == 1)
+		return ret;
+
+	if (buf[0] == MIPI_DCS_WRITE_MEMORY_START)
+		bpw = 16;
+
+	gpiod_set_value_cansleep(dbi->dc, 1);
+	speed_hz = mipi_dbi_spi_cmd_max_speed(spi, msg->tx_len - 1);
+
+	ret = mipi_dbi_spi_transfer(spi, speed_hz, bpw,
+				    &buf[1], msg->tx_len - 1);
+	if (ret)
+		return ret;
+
+	return msg->tx_len;
+}
+
+static int dbi_spi_attach(struct mipi_dsi_host *host,
+			  struct mipi_dsi_device *dsi)
+{
+	return 0; /* Nothing to do */
+}
+
+static int dbi_spi_detach(struct mipi_dsi_host *host,
+			  struct mipi_dsi_device *dsi)
+{
+	return 0; /* Nothing to do */
+}
+
+static void dbi_spi_host_unregister(void *d)
+{
+	mipi_dsi_host_unregister(d);
+}
+
+static int dbi_spi_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mipi_dsi_device_info info = { };
+	struct mipi_dsi_device *dsi;
+	struct dbi_spi *dbi;
+	int ret;
+
+	dbi = devm_kzalloc(dev, sizeof(*dbi), GFP_KERNEL);
+	if (!dbi)
+		return -ENOMEM;
+
+	/* Only DBI Type C3 supported for now */
+	dbi->host.bus_types = MIPI_DEVICE_TYPE_DBI_SPI_MODE3;
+
+	dbi->host.dev = dev;
+	dbi->host.ops = &dbi->host_ops;
+	dbi->spi = spi;
+	spi_set_drvdata(spi, dbi);
+
+	dbi->dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dbi->dc)) {
+		dev_err(dev, "Failed to get gpio 'dc'\n");
+		return PTR_ERR(dbi->dc);
+	}
+
+	dbi->host_ops.transfer = dbi_spi_transfer;
+	dbi->host_ops.attach = dbi_spi_attach;
+	dbi->host_ops.detach = dbi_spi_detach;
+
+	mutex_init(&dbi->cmdlock);
+
+	ret = mipi_dsi_host_register(&dbi->host);
+	if (ret) {
+		dev_err(dev, "Unable to register DSI host\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, dbi_spi_host_unregister, &dbi->host);
+	if (ret)
+		return ret;
+
+	/*
+	 * Register our own node as a MIPI DSI device.
+	 * This ensures that the panel driver will be probed.
+	 */
+	info.channel = 0;
+	info.node = of_node_get(dev->of_node);
+
+	dsi = mipi_dsi_device_register_full(&dbi->host, &info);
+	if (IS_ERR(dsi)) {
+		dev_err(dev, "Failed to add DSI device\n");
+		return PTR_ERR(dsi);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id dbi_spi_of_match[] = {
+	{ .compatible = "adafruit,yx240qv29" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dbi_spi_of_match);
+
+static struct spi_driver dbi_spi_driver = {
+	.driver = {
+		.name = "dbi-spi",
+		.of_match_table = dbi_spi_of_match,
+	},
+	.probe = dbi_spi_probe,
+};
+module_spi_driver(dbi_spi_driver);
+
+MODULE_DESCRIPTION("DBI SPI bus driver");
+MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_LICENSE("GPL");
-- 
2.26.2

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

[RFC PATCH 3/4] gpu/drm: Add TinyDRM for DSI/DBI panels

[Paul Cercueil]

The new API function mipi_dsi_maybe_register_tiny_driver() is supposed
to be called by DSI/DBI panel drivers at the end of their probe.

If it is detected that the panel is not connected to any controller,
because it has no port #0 node in Device Tree that points back to it,
then a TinyDRM driver is registered with it.

This TinyDRM driver expects that a DCS-compliant protocol is used by the
DSI/DBI panel and can only be used with these.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/tiny/Kconfig    |   8 +
 drivers/gpu/drm/tiny/Makefile   |   1 +
 drivers/gpu/drm/tiny/tiny-dsi.c | 262 ++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h      |  19 +++
 4 files changed, 290 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/tiny-dsi.c

diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 4160e74e4751..54ee58aecf66 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -9,6 +9,14 @@ config DRM_GM12U320
 	 This is a KMS driver for projectors which use the GM12U320 chipset
 	 for video transfer over USB2/3, such as the Acer C120 mini projector.
 
+config TINYDRM_DSI
+	tristate "DRM support for generic DBI/DSI display panels"
+	depends on DRM && DRM_MIPI_DSI
+	select DRM_MIPI_DBI
+	select DRM_KMS_CMA_HELPER
+	help
+	  DRM driver for generic DBI/DSI display panels
+
 config TINYDRM_HX8357D
 	tristate "DRM support for HX8357D display panels"
 	depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index c96ceee71453..49513db9a307 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
+obj-$(CONFIG_TINYDRM_DSI)		+= tiny-dsi.o
 obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
 obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
 obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
diff --git a/drivers/gpu/drm/tiny/tiny-dsi.c b/drivers/gpu/drm/tiny/tiny-dsi.c
new file mode 100644
index 000000000000..915e598844bd
--- /dev/null
+++ b/drivers/gpu/drm/tiny/tiny-dsi.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * TinyDRM driver for standard DSI/DBI panels
+ *
+ * Copyright 2020 Paul Cercueil <paul@crapouillou.net>
+ */
+
+#include <linux/module.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modeset_helper.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+#include <video/mipi_display.h>
+
+struct tiny_dsi {
+	struct drm_device drm;
+	struct drm_connector connector;
+	struct drm_simple_display_pipe pipe;
+
+	struct mipi_dsi_device *dsi;
+	struct drm_panel *panel;
+};
+
+#define mipi_dcs_command(dsi, cmd, seq...) \
+({ \
+	u8 d[] = { seq }; \
+	mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
+})
+
+static inline struct tiny_dsi *drm_to_tiny_dsi(struct drm_device *drm)
+{
+	return container_of(drm, struct tiny_dsi, drm);
+}
+
+static void tiny_dsi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
+{
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem);
+	struct tiny_dsi *priv = drm_to_tiny_dsi(fb->dev);
+	unsigned int height = rect->y2 - rect->y1;
+	unsigned int width = rect->x2 - rect->x1;
+	bool fb_convert;
+	int idx, ret;
+	void *tr;
+
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
+	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
+
+	fb_convert = width != fb->width || height != fb->height
+		|| fb->format->format == DRM_FORMAT_XRGB8888;
+	if (fb_convert) {
+		tr = kzalloc(width * height * 2, GFP_KERNEL);
+
+		/* TODO: swap pixels if needed */
+		ret = mipi_dbi_buf_copy(tr, fb, rect, false);
+		if (ret)
+			goto err_msg;
+	} else {
+		tr = cma_obj->vaddr;
+	}
+
+	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_COLUMN_ADDRESS,
+			 (rect->x1 >> 8) & 0xff, rect->x1 & 0xff,
+			 (rect->x2 >> 8) & 0xff, rect->x2 & 0xff);
+	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_PAGE_ADDRESS,
+			 (rect->y1 >> 8) & 0xff, rect->y1 & 0xff,
+			 (rect->y2 >> 8) & 0xff, rect->y2 & 0xff);
+
+	ret = mipi_dsi_dcs_write(priv->dsi, MIPI_DCS_WRITE_MEMORY_START,
+				 tr, width * height * 2);
+err_msg:
+	if (ret)
+		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
+
+	if (fb_convert)
+		kfree(tr);
+	drm_dev_exit(idx);
+}
+
+static void tiny_dsi_enable(struct drm_simple_display_pipe *pipe,
+			    struct drm_crtc_state *crtc_state,
+			    struct drm_plane_state *plane_state)
+{
+	struct tiny_dsi *priv = drm_to_tiny_dsi(pipe->crtc.dev);
+
+	drm_panel_enable(priv->panel);
+}
+
+static void tiny_dsi_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct tiny_dsi *priv = drm_to_tiny_dsi(pipe->crtc.dev);
+
+	drm_panel_disable(priv->panel);
+}
+
+static void tiny_dsi_update(struct drm_simple_display_pipe *pipe,
+			    struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_rect rect;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		tiny_dsi_fb_dirty(state->fb, &rect);
+}
+
+static const struct drm_simple_display_pipe_funcs tiny_dsi_pipe_funcs = {
+	.enable = tiny_dsi_enable,
+	.disable = tiny_dsi_disable,
+	.update = tiny_dsi_update,
+	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+
+static int tiny_dsi_connector_get_modes(struct drm_connector *connector)
+{
+	struct tiny_dsi *priv = drm_to_tiny_dsi(connector->dev);
+
+	return drm_panel_get_modes(priv->panel, connector);
+}
+
+static const struct drm_connector_helper_funcs tiny_dsi_connector_hfuncs = {
+	.get_modes = tiny_dsi_connector_get_modes,
+};
+
+static const struct drm_connector_funcs tiny_dsi_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(tiny_dsi_fops);
+
+static const uint32_t tiny_dsi_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const struct drm_mode_config_funcs tiny_dsi_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static void tiny_dsi_release(struct drm_device *drm)
+{
+	struct tiny_dsi *priv = drm_to_tiny_dsi(drm);
+
+	drm_mode_config_cleanup(drm);
+	drm_dev_fini(drm);
+	kfree(priv);
+}
+
+static struct drm_driver tiny_dsi_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.fops			= &tiny_dsi_fops,
+	.release		= tiny_dsi_release,
+	DRM_GEM_CMA_VMAP_DRIVER_OPS,
+	.name			= "tiny-dsi",
+	.desc			= "Tiny DSI",
+	.date			= "20200605",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static void tiny_dsi_remove(void *drm)
+{
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+}
+
+int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct drm_device *drm;
+	struct tiny_dsi *priv;
+	static const uint64_t modifiers[] = {
+		DRM_FORMAT_MOD_LINEAR,
+		DRM_FORMAT_MOD_INVALID
+	};
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dsi = dsi;
+	drm = &priv->drm;
+
+	ret = devm_drm_dev_init(dev, drm, &tiny_dsi_driver);
+	if (ret) {
+		kfree(priv);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
+	priv->panel = of_drm_find_panel(dev->of_node);
+	if (IS_ERR(priv->panel)) {
+		dev_err(dev, "Unable to find panel\n");
+		return PTR_ERR(priv->panel);
+	}
+
+	drm->mode_config.preferred_depth = 16;
+
+	drm->mode_config.funcs = &tiny_dsi_mode_config_funcs;
+	drm->mode_config.min_width = 0;
+	drm->mode_config.min_height = 0;
+	drm->mode_config.max_width = 4096;
+	drm->mode_config.max_height = 4096;
+
+	drm_connector_helper_add(&priv->connector, &tiny_dsi_connector_hfuncs);
+	ret = drm_connector_init(drm, &priv->connector, &tiny_dsi_connector_funcs,
+				 DRM_MODE_CONNECTOR_DSI);
+	if (ret) {
+		dev_err(dev, "Unable to init connector\n");
+		return ret;
+	}
+
+	ret = drm_simple_display_pipe_init(drm, &priv->pipe, &tiny_dsi_pipe_funcs,
+					   tiny_dsi_formats, ARRAY_SIZE(tiny_dsi_formats),
+					   modifiers, &priv->connector);
+	if (ret) {
+		dev_err(dev, "Unable to init display pipe\n");
+		return ret;
+	}
+
+	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
+
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret) {
+		dev_err(dev, "Failed to register DRM driver\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, tiny_dsi_remove, drm);
+	if (ret)
+		return ret;
+
+	drm_fbdev_generic_setup(drm, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mipi_dsi_register_tiny_driver);
+
+MODULE_DESCRIPTION("DSI/DBI TinyDRM driver");
+MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_LICENSE("GPL");
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 65d2961fc054..0c2589a55df6 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -10,6 +10,7 @@
 #define __DRM_MIPI_DSI_H__
 
 #include <linux/device.h>
+#include <linux/of_graph.h>
 
 struct mipi_dsi_host;
 struct mipi_dsi_device;
@@ -337,4 +338,22 @@ void mipi_dsi_driver_unregister(struct mipi_dsi_driver *driver);
 	module_driver(__mipi_dsi_driver, mipi_dsi_driver_register, \
 			mipi_dsi_driver_unregister)
 
+#if IS_ENABLED(CONFIG_TINYDRM_DSI)
+int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi);
+#else
+static inline int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi)
+{
+	return 0;
+}
+#endif
+
+static inline int mipi_dsi_maybe_register_tiny_driver(struct mipi_dsi_device *dsi)
+{
+	/* Register the TinyDRM DSI/DBI driver if the panel has no controller */
+	if (!of_graph_get_port_by_id(dsi->dev.of_node, 0))
+		return mipi_dsi_register_tiny_driver(dsi);
+
+	return 0;
+}
+
 #endif /* __DRM_MIPI_DSI__ */
-- 
2.26.2

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

[RFC PATCH 4/4] gpu/drm: Add Ilitek ILI9341 DBI panel driver

[Paul Cercueil]

This driver is for the Ilitek ILI9341 based YX240QV29-T 2.4" 240x320 TFT
LCD panel from Adafruit.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/Kconfig                |   9 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 347 +++++++++++++++++++
 3 files changed, 357 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a1723c1b5fbf..2eb2ca77cab2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -95,6 +95,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_IL9341
+	tristate "Ilitek ILI9341 320x240 QVGA panels"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for Ilitek IL9341
+	  QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
+
 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 96a883cd6630..d1235431691a 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o
 obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.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_IL9341) += panel-ilitek-ili9341.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
 obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
new file mode 100644
index 000000000000..6fb253b49f8a
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <david@lechnology.com>
+ * Copyright 2020 Paul Cercueil <paul@crapouillou.net>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/dma-buf.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/property.h>
+#include <drm/drm_atomic_helper.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <video/mipi_display.h>
+
+#define ILI9341_FRMCTR1		0xb1
+#define ILI9341_DISCTRL		0xb6
+#define ILI9341_ETMOD		0xb7
+
+#define ILI9341_PWCTRL1		0xc0
+#define ILI9341_PWCTRL2		0xc1
+#define ILI9341_VMCTRL1		0xc5
+#define ILI9341_VMCTRL2		0xc7
+#define ILI9341_PWCTRLA		0xcb
+#define ILI9341_PWCTRLB		0xcf
+
+#define ILI9341_PGAMCTRL	0xe0
+#define ILI9341_NGAMCTRL	0xe1
+#define ILI9341_DTCTRLA		0xe8
+#define ILI9341_DTCTRLB		0xea
+#define ILI9341_PWRSEQ		0xed
+
+#define ILI9341_EN3GAM		0xf2
+#define ILI9341_PUMPCTRL	0xf7
+
+#define ILI9341_MADCTL_BGR	BIT(3)
+#define ILI9341_MADCTL_MV	BIT(5)
+#define ILI9341_MADCTL_MX	BIT(6)
+#define ILI9341_MADCTL_MY	BIT(7)
+
+struct ili9341_pdata {
+	struct drm_display_mode mode;
+	unsigned int width_mm;
+	unsigned int height_mm;
+	unsigned int bus_type;
+	unsigned int lanes;
+};
+
+struct ili9341 {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+	const struct ili9341_pdata *pdata;
+
+	struct gpio_desc	*reset_gpiod;
+	struct backlight_device *backlight;
+	u32 rotation;
+};
+
+#define mipi_dcs_command(dsi, cmd, seq...) \
+({ \
+	u8 d[] = { seq }; \
+	mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
+})
+
+static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
+{
+	return container_of(panel, struct ili9341, panel);
+}
+
+static int ili9341_prepare(struct drm_panel *panel)
+{
+	struct ili9341 *priv = panel_to_ili9341(panel);
+	struct mipi_dsi_device *dsi = priv->dsi;
+	u8 addr_mode;
+	int ret;
+
+	gpiod_set_value_cansleep(priv->reset_gpiod, 0);
+	usleep_range(20, 1000);
+	gpiod_set_value_cansleep(priv->reset_gpiod, 1);
+	msleep(120);
+
+	ret = mipi_dcs_command(dsi, MIPI_DCS_SOFT_RESET);
+	if (ret) {
+		dev_err(panel->dev, "Failed to send reset command: %d\n", ret);
+		return ret;
+	}
+
+	/* Wait 5ms after soft reset per MIPI DCS spec */
+	usleep_range(5000, 20000);
+
+	mipi_dcs_command(dsi, MIPI_DCS_SET_DISPLAY_OFF);
+
+	mipi_dcs_command(dsi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+	mipi_dcs_command(dsi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+	mipi_dcs_command(dsi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+	mipi_dcs_command(dsi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+	mipi_dcs_command(dsi, ILI9341_PUMPCTRL, 0x20);
+	mipi_dcs_command(dsi, ILI9341_DTCTRLB, 0x00, 0x00);
+
+	/* Power Control */
+	mipi_dcs_command(dsi, ILI9341_PWCTRL1, 0x23);
+	mipi_dcs_command(dsi, ILI9341_PWCTRL2, 0x10);
+	/* VCOM */
+	mipi_dcs_command(dsi, ILI9341_VMCTRL1, 0x3e, 0x28);
+	mipi_dcs_command(dsi, ILI9341_VMCTRL2, 0x86);
+
+	/* Memory Access Control */
+	mipi_dcs_command(dsi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
+
+	/* Frame Rate */
+	mipi_dcs_command(dsi, ILI9341_FRMCTR1, 0x00, 0x1b);
+
+	/* Gamma */
+	mipi_dcs_command(dsi, ILI9341_EN3GAM, 0x00);
+	mipi_dcs_command(dsi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+	mipi_dcs_command(dsi, ILI9341_PGAMCTRL,
+			 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+			 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+	mipi_dcs_command(dsi, ILI9341_NGAMCTRL,
+			 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+			 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+	/* DDRAM */
+	mipi_dcs_command(dsi, ILI9341_ETMOD, 0x07);
+
+	/* Display */
+	mipi_dcs_command(dsi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
+	mipi_dcs_command(dsi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(100);
+
+	mipi_dcs_command(dsi, MIPI_DCS_SET_DISPLAY_ON);
+	msleep(100);
+
+	switch (priv->rotation) {
+	default:
+		addr_mode = ILI9341_MADCTL_MX;
+		break;
+	case 90:
+		addr_mode = ILI9341_MADCTL_MV;
+		break;
+	case 180:
+		addr_mode = ILI9341_MADCTL_MY;
+		break;
+	case 270:
+		addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
+			    ILI9341_MADCTL_MX;
+		break;
+	}
+	addr_mode |= ILI9341_MADCTL_BGR;
+	mipi_dcs_command(dsi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+
+	return 0;
+}
+
+static int ili9341_enable(struct drm_panel *panel)
+{
+	struct ili9341 *priv = panel_to_ili9341(panel);
+
+	backlight_enable(priv->backlight);
+
+	return 0;
+}
+
+static int ili9341_disable(struct drm_panel *panel)
+{
+	struct ili9341 *priv = panel_to_ili9341(panel);
+
+	backlight_disable(priv->backlight);
+
+	return 0;
+}
+
+static int ili9341_unprepare(struct drm_panel *panel)
+{
+	struct ili9341 *priv = panel_to_ili9341(panel);
+
+	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_DISPLAY_OFF);
+
+	return 0;
+}
+
+static int ili9341_get_modes(struct drm_panel *panel,
+			     struct drm_connector *connector)
+{
+	struct ili9341 *priv = panel_to_ili9341(panel);
+	struct drm_display_mode *mode;
+	u32 format = MEDIA_BUS_FMT_RGB565_1X16;
+
+	mode = drm_mode_duplicate(connector->dev, &priv->pdata->mode);
+	if (!mode) {
+		dev_err(panel->dev, "failed to add mode %ux%ux@%u\n",
+			priv->pdata->mode.hdisplay,
+			priv->pdata->mode.vdisplay,
+			priv->pdata->mode.vrefresh);
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = priv->pdata->width_mm;
+	connector->display_info.height_mm = priv->pdata->height_mm;
+
+	drm_display_info_set_bus_formats(&connector->display_info, &format, 1);
+	connector->display_info.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ili9341_funcs = {
+	.prepare	= ili9341_prepare,
+	.unprepare	= ili9341_unprepare,
+	.enable		= ili9341_enable,
+	.disable	= ili9341_disable,
+	.get_modes	= ili9341_get_modes,
+};
+
+static int ili9341_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct ili9341 *priv;
+	int ret;
+
+	/* See comment for mipi_dbi_spi_init() */
+	if (!dev->coherent_dma_mask) {
+		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret) {
+			dev_warn(dev, "Failed to set dma mask %d\n", ret);
+			return ret;
+		}
+	}
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, priv);
+	priv->dsi = dsi;
+
+	device_property_read_u32(dev, "rotation", &priv->rotation);
+
+	priv->pdata = device_get_match_data(dev);
+	if (!priv->pdata)
+		return -EINVAL;
+
+	drm_panel_init(&priv->panel, dev, &ili9341_funcs,
+		       DRM_MODE_CONNECTOR_DPI);
+
+	priv->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset_gpiod)) {
+		dev_err(dev, "Couldn't get our reset GPIO\n");
+		return PTR_ERR(priv->reset_gpiod);
+	}
+
+	priv->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(priv->backlight)) {
+		ret = PTR_ERR(priv->backlight);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get backlight handle\n");
+		return ret;
+	}
+
+	ret = drm_panel_add(&priv->panel);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register panel\n");
+		return ret;
+	}
+
+	dsi->bus_type = priv->pdata->bus_type;
+	dsi->lanes = priv->pdata->lanes;
+	dsi->format = MIPI_DSI_FMT_RGB565;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret) {
+		dev_err(dev, "Failed to attach DSI panel\n");
+		goto err_panel_remove;
+	}
+
+	ret = mipi_dsi_maybe_register_tiny_driver(dsi);
+	if (ret) {
+		dev_err(dev, "Failed to init TinyDRM driver\n");
+		goto err_mipi_dsi_detach;
+	}
+
+	return 0;
+
+err_mipi_dsi_detach:
+	mipi_dsi_detach(dsi);
+err_panel_remove:
+	drm_panel_remove(&priv->panel);
+	return ret;
+}
+
+static int ili9341_remove(struct mipi_dsi_device *dsi)
+{
+	struct ili9341 *priv = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&priv->panel);
+
+	if (priv->backlight)
+		put_device(&priv->backlight->dev);
+
+	return 0;
+}
+
+static const struct ili9341_pdata yx240qv29_pdata = {
+	.mode = { DRM_SIMPLE_MODE(240, 320, 37, 49) },
+	.width_mm = 0, // TODO
+	.height_mm = 0, // TODO
+	.bus_type = MIPI_DEVICE_TYPE_DBI_SPI_MODE3,
+	.lanes = 1,
+};
+
+static const struct of_device_id ili9341_of_match[] = {
+	{ .compatible = "adafruit,yx240qv29", .data = &yx240qv29_pdata },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ili9341_of_match);
+
+static struct mipi_dsi_driver ili9341_dsi_driver = {
+	.probe		= ili9341_probe,
+	.remove		= ili9341_remove,
+	.driver = {
+		.name		= "ili9341-dsi",
+		.of_match_table	= ili9341_of_match,
+	},
+};
+module_mipi_dsi_driver(ili9341_dsi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9341 DRM panel driver");
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_LICENSE("GPL");
-- 
2.26.2

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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Noralf Trønnes]

Den 07.06.2020 15.38, skrev Paul Cercueil:
> Hi,
> 
> Here's a follow-up on the previous discussion about the current state of
> DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
> 
> This patchset introduces the following:
> * It slightly tweaks the MIPI DSI code so that it supports MIPI DBI over
>   various buses. This patch has been tested with a non-upstream DRM
>   panel driver for a ILI9331 DBI/8080 panel, written with the DSI
>   framework (and doesn't include <drm/drm_mipi_dbi.h>), and non-upstream
>   DSI/DBI host driver for the Ingenic SoCs.
> 
> * A SPI DBI host driver, using the current MIPI DSI framework. It allows
>   MIPI DSI/DBI drivers to be written with the DSI framework, even if
>   they are connected over SPI, instead of registering as SPI device
>   drivers. Since most of these panels can be connected over various
>   buses, it permits to reuse the same driver independently of the bus
>   used.
> 
> * A TinyDRM driver for DSI/DBI panels, once again independent of the bus
>   used; the only dependency (currently) being that the panel must
>   understand DCS commands.
> 
> * A DRM panel driver to test the stack. This driver controls Ilitek
>   ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
>   TFT LCD panel. This panel was converted from
>   drivers/gpu/drm/tiny/ili9341.c.
> 
> I would like to emphasize that while it has been compile-tested, I did
> not test it with real hardware since I do not have any DBI panel
> connected over SPI. I did runtime-test the code, just without any panel
> connected.
> 
> Another thing to note, is that it does not break Device Tree ABI. The
> display node stays the same:
> 
> display@0 {
> 	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
> 	reg = <0>;
> 	spi-max-frequency = <32000000>;
> 	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> 	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> 	rotation = <270>;
> 	backlight = <&backlight>;
> };
> 
> The reason it works, is that the "adafruit,yx240qv29" device is probed
> on the SPI bus, so it will match with the SPI/DBI host driver. This will
> in turn register the very same node with the DSI bus, and the ILI9341
> DRM panel driver will probe. The driver will detect that no controller
> is linked to the panel, and eventually register the DBI/DSI TinyDRM
> driver.
> 
> I can't stress it enough that this is a RFC, so it still has very rough
> edges.
> 

I don't know bridge and dsi drivers so I can't comment on that, but one
thing I didn't like is that the DT compatible string has to be added to
2 different modules.

As an example, a MI0283QT panel (ILI9341) supports these interface options:

1. SPI
   Panel setup/control and framebuffer upload over SPI

2. SPI + DPI
   Panel setup/control over SPI, framebuffer scanout over DPI

3. Parallel bus
   Panel setup/control and framebuffer upload over parallel bus

My suggestion is to have one panel driver module that can support all of
these like this:

For 1. and 2. a SPI driver is registered and if I understand your
example correctly of_graph_get_port_by_id() can be used during probe to
distinguish between the 2 options and register a full DRM driver for 1.
and add a DRM panel for 2.

For 3. a DSI driver is registered (adapted for DBI use like you're
suggesting).

Note that the interface part of the controller initialization will
differ between these, the panel side init will be the same I assume.

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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Paul Cercueil]

Hi Noralf,


Le dim. 14 juin 2020 à 18:36, Noralf Trønnes <noralf@tronnes.org> a 
écrit :
> 
> 
> Den 07.06.2020 15.38, skrev Paul Cercueil:
>>  Hi,
>> 
>>  Here's a follow-up on the previous discussion about the current 
>> state of
>>  DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
>> 
>>  This patchset introduces the following:
>>  * It slightly tweaks the MIPI DSI code so that it supports MIPI DBI 
>> over
>>    various buses. This patch has been tested with a non-upstream DRM
>>    panel driver for a ILI9331 DBI/8080 panel, written with the DSI
>>    framework (and doesn't include <drm/drm_mipi_dbi.h>), and 
>> non-upstream
>>    DSI/DBI host driver for the Ingenic SoCs.
>> 
>>  * A SPI DBI host driver, using the current MIPI DSI framework. It 
>> allows
>>    MIPI DSI/DBI drivers to be written with the DSI framework, even if
>>    they are connected over SPI, instead of registering as SPI device
>>    drivers. Since most of these panels can be connected over various
>>    buses, it permits to reuse the same driver independently of the 
>> bus
>>    used.
>> 
>>  * A TinyDRM driver for DSI/DBI panels, once again independent of 
>> the bus
>>    used; the only dependency (currently) being that the panel must
>>    understand DCS commands.
>> 
>>  * A DRM panel driver to test the stack. This driver controls Ilitek
>>    ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 
>> 2.4"
>>    TFT LCD panel. This panel was converted from
>>    drivers/gpu/drm/tiny/ili9341.c.
>> 
>>  I would like to emphasize that while it has been compile-tested, I 
>> did
>>  not test it with real hardware since I do not have any DBI panel
>>  connected over SPI. I did runtime-test the code, just without any 
>> panel
>>  connected.
>> 
>>  Another thing to note, is that it does not break Device Tree ABI. 
>> The
>>  display node stays the same:
>> 
>>  display@0 {
>>  	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
>>  	reg = <0>;
>>  	spi-max-frequency = <32000000>;
>>  	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>>  	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>>  	rotation = <270>;
>>  	backlight = <&backlight>;
>>  };
>> 
>>  The reason it works, is that the "adafruit,yx240qv29" device is 
>> probed
>>  on the SPI bus, so it will match with the SPI/DBI host driver. This 
>> will
>>  in turn register the very same node with the DSI bus, and the 
>> ILI9341
>>  DRM panel driver will probe. The driver will detect that no 
>> controller
>>  is linked to the panel, and eventually register the DBI/DSI TinyDRM
>>  driver.
>> 
>>  I can't stress it enough that this is a RFC, so it still has very 
>> rough
>>  edges.
>> 
> 
> I don't know bridge and dsi drivers so I can't comment on that, but 
> one
> thing I didn't like is that the DT compatible string has to be added 
> to
> 2 different modules.

That's a minimal drawback for a perfectly architectured design ;)

> As an example, a MI0283QT panel (ILI9341) supports these interface 
> options:
> 
> 1. SPI
>    Panel setup/control and framebuffer upload over SPI
> 
> 2. SPI + DPI
>    Panel setup/control over SPI, framebuffer scanout over DPI
> 
> 3. Parallel bus
>    Panel setup/control and framebuffer upload over parallel bus
> 
> My suggestion is to have one panel driver module that can support all 
> of
> these like this:
> 
> For 1. and 2. a SPI driver is registered and if I understand your
> example correctly of_graph_get_port_by_id() can be used during probe 
> to
> distinguish between the 2 options and register a full DRM driver for 
> 1.
> and add a DRM panel for 2.
> 
> For 3. a DSI driver is registered (adapted for DBI use like you're
> suggesting).

That means basically having two entry points per DBI driver, one as DSI 
device and one as SPI device, the latter doing the job of the current 
DBI-SPI bridge. I think i would be cleaner to just have duplicated 
strings with the DBI-SPI bridge.

Cheers,
-Paul

> Note that the interface part of the controller initialization will
> differ between these, the panel side init will be the same I assume.

> Noralf.


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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Emil Velikov]

Hi all,

Allow me to compare this approach with some work Linus W [1] did a
while back, which I've just noticed.

Pauls' approach:

 - Perhaps the shortest one possible
Porting an existing DSI panel to DBI is 3 lines of code - compat line
in the SPI/DSI bridge, a bus_type and
mipi_dsi_maybe_register_tiny_driver() call
The clever use of the DSI type (equal to zero) means that things will
work w/o updating existing dsi hosts and devices in panel/. Yet in the
very unlikely case that the panel does not support DSI, we will still
allow it.

Although thinking about the type I wonder if it can accommodate all use-cases:
Since we can have a device (panel) capable of DSI+SPI it makes sense
for it to expose the type bitmask, not the host. Although, what if the
host itself supports DSI+SPI.?
Now we can extrapolate that with a host (say fimd/exynos I think)
which supports a SPI panel (s6e63m0) while having
of_graph_get_port_by_id(0)?

- Strange (ab)use of the DSI bus for DBI (SPI, 6800, 8080 etc)
We care about existing users (DT) and it sounds unlikely (based on
previous discussion) that DBI + SPI/6800... will make it into DT. So
the current approach seems quite reasonable IMHO.


Linus' approach:
- Clear separation of DSI/SPI
Compat strings are still duplicated, although in separate files

- Minor code motion and slightly more invasive porting overall
Much of the boilerplate can be reduced via helper lib and macros. Even
then it's unlikely we'll reach the 3 line delta as with Paul's
approach.

- Does not handle tiny-dsi (dummy) drm driver
It seems doable, with minor changes


Personally I'm on the fence and a deciding factor for me is if Paul's
approach can handle all the combinations of host/device type support.
That said, the input from people likely to do the work would be highly
appreciated.

Once a decision is made, an illustrative series + todo entry would be
great to have.
-Emil

[1] https://lists.freedesktop.org/archives/dri-devel/2020-May/266079.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Paul Cercueil]

Hi Emil,

Le mar. 16 juin 2020 à 18:47, Emil Velikov <emil.l.velikov@gmail.com> 
a écrit :
> Hi all,
> 
> Allow me to compare this approach with some work Linus W [1] did a
> while back, which I've just noticed.
> 
> Pauls' approach:
> 
>  - Perhaps the shortest one possible
> Porting an existing DSI panel to DBI is 3 lines of code - compat line
> in the SPI/DSI bridge, a bus_type and
> mipi_dsi_maybe_register_tiny_driver() call
> The clever use of the DSI type (equal to zero) means that things will
> work w/o updating existing dsi hosts and devices in panel/. Yet in the
> very unlikely case that the panel does not support DSI, we will still
> allow it.

Is there such a case? I assumed that all current DSI device and host 
drivers are for DSI panels.

> Although thinking about the type I wonder if it can accommodate all 
> use-cases:
> Since we can have a device (panel) capable of DSI+SPI it makes sense
> for it to expose the type bitmask, not the host. Although, what if the
> host itself supports DSI+SPI.?

Yes, hosts can support more than one type (mine does), so it should 
expose a bitmask. As for the panel, even though some can do DSI, SPI 
and I8080 DBI, as far as I know the bus used is always set in hardware 
(with specific pins set to VCC/GND to select the mode), so this is not 
something the host can select. Therefore, the panel driver should 
register the mipi_dsi_device with one particular bus type. Note that 
the panel driver could very well infer the bus type from the compatible 
string.

If the bus type can be changed at runtime (and there's a need for 
that), then we would need a bitmask on the panel driver side too, along 
with a 'set bus' callback, but I'm not sure it will be required.

> Now we can extrapolate that with a host (say fimd/exynos I think)
> which supports a SPI panel (s6e63m0) while having
> of_graph_get_port_by_id(0)?

I'm not sure I understand, if there is a port #0 in the panel node, 
then the tinyDRM driver is not created, and the SPI panel will be 
registered with the fimd/exynos host driver. So that should already 
work fine.

> - Strange (ab)use of the DSI bus for DBI (SPI, 6800, 8080 etc)
> We care about existing users (DT) and it sounds unlikely (based on
> previous discussion) that DBI + SPI/6800... will make it into DT. So
> the current approach seems quite reasonable IMHO.
> 
> 
> Linus' approach:
> - Clear separation of DSI/SPI
> Compat strings are still duplicated, although in separate files
> 
> - Minor code motion and slightly more invasive porting overall
> Much of the boilerplate can be reduced via helper lib and macros. Even
> then it's unlikely we'll reach the 3 line delta as with Paul's
> approach.
> 
> - Does not handle tiny-dsi (dummy) drm driver
> It seems doable, with minor changes
> 
> 
> Personally I'm on the fence and a deciding factor for me is if Paul's
> approach can handle all the combinations of host/device type support.
> That said, the input from people likely to do the work would be highly
> appreciated.
> 
> Once a decision is made, an illustrative series + todo entry would be
> great to have.

Sure. Thanks for the feedback!

Cheers,
-Paul

> -Emil
> 
> [1] 
> https://lists.freedesktop.org/archives/dri-devel/2020-May/266079.html


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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Paul Cercueil]

Hi Emil,

Le mar. 16 juin 2020 à 18:47, Emil Velikov <emil.l.velikov@gmail.com> 
a écrit :
> Hi all,
> 
> Allow me to compare this approach with some work Linus W [1] did a
> while back, which I've just noticed.
> 
> Pauls' approach:
> 
>  - Perhaps the shortest one possible
> Porting an existing DSI panel to DBI is 3 lines of code - compat line
> in the SPI/DSI bridge, a bus_type and
> mipi_dsi_maybe_register_tiny_driver() call
> The clever use of the DSI type (equal to zero) means that things will
> work w/o updating existing dsi hosts and devices in panel/. Yet in the
> very unlikely case that the panel does not support DSI, we will still
> allow it.

Actually the DSI type is not equal to zero, it's BIT(0) so 1. Right now 
in the patch, I added a WARN in mipi_dsi_attach() that checks that 
bus_type is non-zero.

-Paul

> Although thinking about the type I wonder if it can accommodate all 
> use-cases:
> Since we can have a device (panel) capable of DSI+SPI it makes sense
> for it to expose the type bitmask, not the host. Although, what if the
> host itself supports DSI+SPI.?
> Now we can extrapolate that with a host (say fimd/exynos I think)
> which supports a SPI panel (s6e63m0) while having
> of_graph_get_port_by_id(0)?
> 
> - Strange (ab)use of the DSI bus for DBI (SPI, 6800, 8080 etc)
> We care about existing users (DT) and it sounds unlikely (based on
> previous discussion) that DBI + SPI/6800... will make it into DT. So
> the current approach seems quite reasonable IMHO.
> 
> 
> Linus' approach:
> - Clear separation of DSI/SPI
> Compat strings are still duplicated, although in separate files
> 
> - Minor code motion and slightly more invasive porting overall
> Much of the boilerplate can be reduced via helper lib and macros. Even
> then it's unlikely we'll reach the 3 line delta as with Paul's
> approach.
> 
> - Does not handle tiny-dsi (dummy) drm driver
> It seems doable, with minor changes
> 
> 
> Personally I'm on the fence and a deciding factor for me is if Paul's
> approach can handle all the combinations of host/device type support.
> That said, the input from people likely to do the work would be highly
> appreciated.
> 
> Once a decision is made, an illustrative series + todo entry would be
> great to have.
> -Emil
> 
> [1] 
> https://lists.freedesktop.org/archives/dri-devel/2020-May/266079.html


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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Sam Ravnborg]

Hi Noralf/Paul.

Trying to stir up this discussion again.

On Sun, Jun 14, 2020 at 06:36:22PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 07.06.2020 15.38, skrev Paul Cercueil:
> > Hi,
> > 
> > Here's a follow-up on the previous discussion about the current state of
> > DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
> > 
> > This patchset introduces the following:
> > * It slightly tweaks the MIPI DSI code so that it supports MIPI DBI over
> >   various buses. This patch has been tested with a non-upstream DRM
> >   panel driver for a ILI9331 DBI/8080 panel, written with the DSI
> >   framework (and doesn't include <drm/drm_mipi_dbi.h>), and non-upstream
> >   DSI/DBI host driver for the Ingenic SoCs.
> > 
> > * A SPI DBI host driver, using the current MIPI DSI framework. It allows
> >   MIPI DSI/DBI drivers to be written with the DSI framework, even if
> >   they are connected over SPI, instead of registering as SPI device
> >   drivers. Since most of these panels can be connected over various
> >   buses, it permits to reuse the same driver independently of the bus
> >   used.
> > 
> > * A TinyDRM driver for DSI/DBI panels, once again independent of the bus
> >   used; the only dependency (currently) being that the panel must
> >   understand DCS commands.
> > 
> > * A DRM panel driver to test the stack. This driver controls Ilitek
> >   ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
> >   TFT LCD panel. This panel was converted from
> >   drivers/gpu/drm/tiny/ili9341.c.
> > 
> > I would like to emphasize that while it has been compile-tested, I did
> > not test it with real hardware since I do not have any DBI panel
> > connected over SPI. I did runtime-test the code, just without any panel
> > connected.
> > 
> > Another thing to note, is that it does not break Device Tree ABI. The
> > display node stays the same:
> > 
> > display@0 {
> > 	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
> > 	reg = <0>;
> > 	spi-max-frequency = <32000000>;
> > 	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> > 	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> > 	rotation = <270>;
> > 	backlight = <&backlight>;
> > };
> > 
> > The reason it works, is that the "adafruit,yx240qv29" device is probed
> > on the SPI bus, so it will match with the SPI/DBI host driver. This will
> > in turn register the very same node with the DSI bus, and the ILI9341
> > DRM panel driver will probe. The driver will detect that no controller
> > is linked to the panel, and eventually register the DBI/DSI TinyDRM
> > driver.
> > 
> > I can't stress it enough that this is a RFC, so it still has very rough
> > edges.
> > 
> 
> I don't know bridge and dsi drivers so I can't comment on that, but one
> thing I didn't like is that the DT compatible string has to be added to
> 2 different modules.
> 
> As an example, a MI0283QT panel (ILI9341) supports these interface options:
> 
> 1. SPI
>    Panel setup/control and framebuffer upload over SPI
> 
> 2. SPI + DPI
>    Panel setup/control over SPI, framebuffer scanout over DPI
> 
> 3. Parallel bus
>    Panel setup/control and framebuffer upload over parallel bus

To continue the configurations we should support:
- Panels where the chip can be configured to SPI, SPI+DPI, Parallel bus
  (as detailed by Noralf above)
- Panels that supports only 6800 or 8080 - connected via GPIO pins or
  memory mapped (maybe behind some special IP to support this)
  Command set is often special.

We will see a number of chips with many different types of displays.
So the drivers should be chip specific with configuration depending on
the connected display.

What I hope we can find a solution for is a single file/driver that can
support all the relevant interface types for a chip.
So we would end up with a single file that included the necessary
support for ili9341 in all interface configurations with the necessary
support for the relevant displays.

I do not know how far we are from this as I have not dived into the
details of any of the proposals.
> 
> My suggestion is to have one panel driver module that can support all of
> these like this:
So I think we agree here.

> 
> For 1. and 2. a SPI driver is registered and if I understand your
> example correctly of_graph_get_port_by_id() can be used during probe to
> distinguish between the 2 options and register a full DRM driver for 1.
> and add a DRM panel for 2.
> 
> For 3. a DSI driver is registered (adapted for DBI use like you're
> suggesting).
> 
> Note that the interface part of the controller initialization will
> differ between these, the panel side init will be the same I assume.

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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Noralf Trønnes]

(cc Dillon)

Den 03.07.2020 19.26, skrev Sam Ravnborg:
> Hi Noralf/Paul.
> 
> Trying to stir up this discussion again.
> 
> On Sun, Jun 14, 2020 at 06:36:22PM +0200, Noralf Trønnes wrote:
>>
>>
>> Den 07.06.2020 15.38, skrev Paul Cercueil:
>>> Hi,
>>>
>>> Here's a follow-up on the previous discussion about the current state of
>>> DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
>>>
>>> This patchset introduces the following:
>>> * It slightly tweaks the MIPI DSI code so that it supports MIPI DBI over
>>>   various buses. This patch has been tested with a non-upstream DRM
>>>   panel driver for a ILI9331 DBI/8080 panel, written with the DSI
>>>   framework (and doesn't include <drm/drm_mipi_dbi.h>), and non-upstream
>>>   DSI/DBI host driver for the Ingenic SoCs.
>>>
>>> * A SPI DBI host driver, using the current MIPI DSI framework. It allows
>>>   MIPI DSI/DBI drivers to be written with the DSI framework, even if
>>>   they are connected over SPI, instead of registering as SPI device
>>>   drivers. Since most of these panels can be connected over various
>>>   buses, it permits to reuse the same driver independently of the bus
>>>   used.
>>>
>>> * A TinyDRM driver for DSI/DBI panels, once again independent of the bus
>>>   used; the only dependency (currently) being that the panel must
>>>   understand DCS commands.
>>>
>>> * A DRM panel driver to test the stack. This driver controls Ilitek
>>>   ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
>>>   TFT LCD panel. This panel was converted from
>>>   drivers/gpu/drm/tiny/ili9341.c.
>>>
>>> I would like to emphasize that while it has been compile-tested, I did
>>> not test it with real hardware since I do not have any DBI panel
>>> connected over SPI. I did runtime-test the code, just without any panel
>>> connected.
>>>
>>> Another thing to note, is that it does not break Device Tree ABI. The
>>> display node stays the same:
>>>
>>> display@0 {
>>> 	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
>>> 	reg = <0>;
>>> 	spi-max-frequency = <32000000>;
>>> 	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>>> 	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>>> 	rotation = <270>;
>>> 	backlight = <&backlight>;
>>> };
>>>
>>> The reason it works, is that the "adafruit,yx240qv29" device is probed
>>> on the SPI bus, so it will match with the SPI/DBI host driver. This will
>>> in turn register the very same node with the DSI bus, and the ILI9341
>>> DRM panel driver will probe. The driver will detect that no controller
>>> is linked to the panel, and eventually register the DBI/DSI TinyDRM
>>> driver.
>>>
>>> I can't stress it enough that this is a RFC, so it still has very rough
>>> edges.
>>>
>>
>> I don't know bridge and dsi drivers so I can't comment on that, but one
>> thing I didn't like is that the DT compatible string has to be added to
>> 2 different modules.
>>
>> As an example, a MI0283QT panel (ILI9341) supports these interface options:
>>
>> 1. SPI
>>    Panel setup/control and framebuffer upload over SPI
>>
>> 2. SPI + DPI
>>    Panel setup/control over SPI, framebuffer scanout over DPI
>>
>> 3. Parallel bus
>>    Panel setup/control and framebuffer upload over parallel bus
> 
> To continue the configurations we should support:
> - Panels where the chip can be configured to SPI, SPI+DPI, Parallel bus
>   (as detailed by Noralf above)
> - Panels that supports only 6800 or 8080 - connected via GPIO pins or
>   memory mapped (maybe behind some special IP to support this)
>   Command set is often special.
> 
> We will see a number of chips with many different types of displays.
> So the drivers should be chip specific with configuration depending on
> the connected display.
> 
> What I hope we can find a solution for is a single file/driver that can
> support all the relevant interface types for a chip.
> So we would end up with a single file that included the necessary
> support for ili9341 in all interface configurations with the necessary
> support for the relevant displays.
> 
> I do not know how far we are from this as I have not dived into the
> details of any of the proposals.

In an ideal world I would have liked to see the MIPI DBI parallel
interface implemented using a new Linux parallel bus type. It could have
drivers for gpio bitbanging and mmio in addition to other hw specific
drivers. Now we could have a drm_mipi_dbi DRM driver that registers as a
SPI client driver and a Parallel bus client driver. Or it can be a
component driver for the existing DRM driver on the SoC.

I had plans to do this and made a prototype, but dropped it since it
would probably require a lot of work getting in a new Linux bus type.

However if we're going to treat this parallel bus only as a MIPI DBI
display interface but support gpio bitbanging and mmio as well, then we
could add DRM drivers for each MIPI DBI bus (that don't have special
parallel bus hw):
- mipi-dbi-spi
- mipi-dbi-gpio
- mipi-dbi-mmio

These drivers will register as a mipi_dsi_host adapted like Paul suggested.

The panel drivers will be mipi_dsi_drivers. Now the panels should work
regardless of bus type. They probably need to know about the bus type,
at least whether the parallell bus is 8-bit or 16-bit wide.

The current MIPI DBI SPI drivers (drm/tiny) will need to be treated
specially to keep working with old Device Trees when moved over to
drm/panel.

Noralf.


>>
>> My suggestion is to have one panel driver module that can support all of
>> these like this:
> So I think we agree here.
> 
>>
>> For 1. and 2. a SPI driver is registered and if I understand your
>> example correctly of_graph_get_port_by_id() can be used during probe to
>> distinguish between the 2 options and register a full DRM driver for 1.
>> and add a DRM panel for 2.
>>
>> For 3. a DSI driver is registered (adapted for DBI use like you're
>> suggesting).
>>
>> Note that the interface part of the controller initialization will
>> differ between these, the panel side init will be the same I assume.
> 
> 	Sam
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 3/4] gpu/drm: Add TinyDRM for DSI/DBI panels

[Sandy Huang]

Hi paul,

     After add this driver, the followinig usage scenarios can be supported?

     panel 1. crtc->encoder->connector[edp/hdmi/mipi dsi] -> panel

     panel 2. Panel setup/control and framebuffer upload over SPI

     the two panel maybe display same/different conctent at same time.


在 2020/6/7 21:38, Paul Cercueil 写道:
> The new API function mipi_dsi_maybe_register_tiny_driver() is supposed
> to be called by DSI/DBI panel drivers at the end of their probe.
>
> If it is detected that the panel is not connected to any controller,
> because it has no port #0 node in Device Tree that points back to it,
> then a TinyDRM driver is registered with it.
>
> This TinyDRM driver expects that a DCS-compliant protocol is used by the
> DSI/DBI panel and can only be used with these.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/tiny/Kconfig    |   8 +
>   drivers/gpu/drm/tiny/Makefile   |   1 +
>   drivers/gpu/drm/tiny/tiny-dsi.c | 262 ++++++++++++++++++++++++++++++++
>   include/drm/drm_mipi_dsi.h      |  19 +++
>   4 files changed, 290 insertions(+)
>   create mode 100644 drivers/gpu/drm/tiny/tiny-dsi.c
>
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index 4160e74e4751..54ee58aecf66 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -9,6 +9,14 @@ config DRM_GM12U320
>   	 This is a KMS driver for projectors which use the GM12U320 chipset
>   	 for video transfer over USB2/3, such as the Acer C120 mini projector.
>   
> +config TINYDRM_DSI
> +	tristate "DRM support for generic DBI/DSI display panels"
> +	depends on DRM && DRM_MIPI_DSI
> +	select DRM_MIPI_DBI
> +	select DRM_KMS_CMA_HELPER
> +	help
> +	  DRM driver for generic DBI/DSI display panels
> +
>   config TINYDRM_HX8357D
>   	tristate "DRM support for HX8357D display panels"
>   	depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index c96ceee71453..49513db9a307 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -1,6 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   
>   obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
> +obj-$(CONFIG_TINYDRM_DSI)		+= tiny-dsi.o
>   obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
>   obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
>   obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
> diff --git a/drivers/gpu/drm/tiny/tiny-dsi.c b/drivers/gpu/drm/tiny/tiny-dsi.c
> new file mode 100644
> index 000000000000..915e598844bd
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/tiny-dsi.c
> @@ -0,0 +1,262 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * TinyDRM driver for standard DSI/DBI panels
> + *
> + * Copyright 2020 Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/module.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_mipi_dbi.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modeset_helper.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct tiny_dsi {
> +	struct drm_device drm;
> +	struct drm_connector connector;
> +	struct drm_simple_display_pipe pipe;
> +
> +	struct mipi_dsi_device *dsi;
> +	struct drm_panel *panel;
> +};
> +
> +#define mipi_dcs_command(dsi, cmd, seq...) \
> +({ \
> +	u8 d[] = { seq }; \
> +	mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
> +})
> +
> +static inline struct tiny_dsi *drm_to_tiny_dsi(struct drm_device *drm)
> +{
> +	return container_of(drm, struct tiny_dsi, drm);
> +}
> +
> +static void tiny_dsi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
> +{
> +	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
> +	struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem);
> +	struct tiny_dsi *priv = drm_to_tiny_dsi(fb->dev);
> +	unsigned int height = rect->y2 - rect->y1;
> +	unsigned int width = rect->x2 - rect->x1;
> +	bool fb_convert;
> +	int idx, ret;
> +	void *tr;
> +
> +	if (!drm_dev_enter(fb->dev, &idx))
> +		return;
> +
> +	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
> +
> +	fb_convert = width != fb->width || height != fb->height
> +		|| fb->format->format == DRM_FORMAT_XRGB8888;
> +	if (fb_convert) {
> +		tr = kzalloc(width * height * 2, GFP_KERNEL);
> +
> +		/* TODO: swap pixels if needed */
> +		ret = mipi_dbi_buf_copy(tr, fb, rect, false);
> +		if (ret)
> +			goto err_msg;
> +	} else {
> +		tr = cma_obj->vaddr;
> +	}
> +
> +	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_COLUMN_ADDRESS,
> +			 (rect->x1 >> 8) & 0xff, rect->x1 & 0xff,
> +			 (rect->x2 >> 8) & 0xff, rect->x2 & 0xff);
> +	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_PAGE_ADDRESS,
> +			 (rect->y1 >> 8) & 0xff, rect->y1 & 0xff,
> +			 (rect->y2 >> 8) & 0xff, rect->y2 & 0xff);
> +
> +	ret = mipi_dsi_dcs_write(priv->dsi, MIPI_DCS_WRITE_MEMORY_START,
> +				 tr, width * height * 2);
> +err_msg:
> +	if (ret)
> +		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
> +
> +	if (fb_convert)
> +		kfree(tr);
> +	drm_dev_exit(idx);
> +}
> +
> +static void tiny_dsi_enable(struct drm_simple_display_pipe *pipe,
> +			    struct drm_crtc_state *crtc_state,
> +			    struct drm_plane_state *plane_state)
> +{
> +	struct tiny_dsi *priv = drm_to_tiny_dsi(pipe->crtc.dev);
> +
> +	drm_panel_enable(priv->panel);
> +}
> +
> +static void tiny_dsi_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct tiny_dsi *priv = drm_to_tiny_dsi(pipe->crtc.dev);
> +
> +	drm_panel_disable(priv->panel);
> +}
> +
> +static void tiny_dsi_update(struct drm_simple_display_pipe *pipe,
> +			    struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = pipe->plane.state;
> +	struct drm_rect rect;
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +		tiny_dsi_fb_dirty(state->fb, &rect);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs tiny_dsi_pipe_funcs = {
> +	.enable = tiny_dsi_enable,
> +	.disable = tiny_dsi_disable,
> +	.update = tiny_dsi_update,
> +	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +};
> +
> +static int tiny_dsi_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct tiny_dsi *priv = drm_to_tiny_dsi(connector->dev);
> +
> +	return drm_panel_get_modes(priv->panel, connector);
> +}
> +
> +static const struct drm_connector_helper_funcs tiny_dsi_connector_hfuncs = {
> +	.get_modes = tiny_dsi_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs tiny_dsi_connector_funcs = {
> +	.reset = drm_atomic_helper_connector_reset,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(tiny_dsi_fops);
> +
> +static const uint32_t tiny_dsi_formats[] = {
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static const struct drm_mode_config_funcs tiny_dsi_mode_config_funcs = {
> +	.fb_create = drm_gem_fb_create_with_dirty,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static void tiny_dsi_release(struct drm_device *drm)
> +{
> +	struct tiny_dsi *priv = drm_to_tiny_dsi(drm);
> +
> +	drm_mode_config_cleanup(drm);
> +	drm_dev_fini(drm);
> +	kfree(priv);
> +}
> +
> +static struct drm_driver tiny_dsi_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.fops			= &tiny_dsi_fops,
> +	.release		= tiny_dsi_release,
> +	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	.name			= "tiny-dsi",
> +	.desc			= "Tiny DSI",
> +	.date			= "20200605",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static void tiny_dsi_remove(void *drm)
> +{
> +	drm_dev_unplug(drm);
> +	drm_atomic_helper_shutdown(drm);
> +}
> +
> +int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct drm_device *drm;
> +	struct tiny_dsi *priv;
> +	static const uint64_t modifiers[] = {
> +		DRM_FORMAT_MOD_LINEAR,
> +		DRM_FORMAT_MOD_INVALID
> +	};
> +	int ret;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dsi = dsi;
> +	drm = &priv->drm;
> +
> +	ret = devm_drm_dev_init(dev, drm, &tiny_dsi_driver);
> +	if (ret) {
> +		kfree(priv);
> +		return ret;
> +	}
> +
> +	drm_mode_config_init(drm);
> +
> +	priv->panel = of_drm_find_panel(dev->of_node);
> +	if (IS_ERR(priv->panel)) {
> +		dev_err(dev, "Unable to find panel\n");
> +		return PTR_ERR(priv->panel);
> +	}
> +
> +	drm->mode_config.preferred_depth = 16;
> +
> +	drm->mode_config.funcs = &tiny_dsi_mode_config_funcs;
> +	drm->mode_config.min_width = 0;
> +	drm->mode_config.min_height = 0;
> +	drm->mode_config.max_width = 4096;
> +	drm->mode_config.max_height = 4096;
> +
> +	drm_connector_helper_add(&priv->connector, &tiny_dsi_connector_hfuncs);
> +	ret = drm_connector_init(drm, &priv->connector, &tiny_dsi_connector_funcs,
> +				 DRM_MODE_CONNECTOR_DSI);
> +	if (ret) {
> +		dev_err(dev, "Unable to init connector\n");
> +		return ret;
> +	}
> +
> +	ret = drm_simple_display_pipe_init(drm, &priv->pipe, &tiny_dsi_pipe_funcs,
> +					   tiny_dsi_formats, ARRAY_SIZE(tiny_dsi_formats),
> +					   modifiers, &priv->connector);
> +	if (ret) {
> +		dev_err(dev, "Unable to init display pipe\n");
> +		return ret;
> +	}
> +
> +	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
> +
> +	drm_mode_config_reset(drm);
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret) {
> +		dev_err(dev, "Failed to register DRM driver\n");
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, tiny_dsi_remove, drm);
> +	if (ret)
> +		return ret;
> +
> +	drm_fbdev_generic_setup(drm, 0);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mipi_dsi_register_tiny_driver);
> +
> +MODULE_DESCRIPTION("DSI/DBI TinyDRM driver");
> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 65d2961fc054..0c2589a55df6 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -10,6 +10,7 @@
>   #define __DRM_MIPI_DSI_H__
>   
>   #include <linux/device.h>
> +#include <linux/of_graph.h>
>   
>   struct mipi_dsi_host;
>   struct mipi_dsi_device;
> @@ -337,4 +338,22 @@ void mipi_dsi_driver_unregister(struct mipi_dsi_driver *driver);
>   	module_driver(__mipi_dsi_driver, mipi_dsi_driver_register, \
>   			mipi_dsi_driver_unregister)
>   
> +#if IS_ENABLED(CONFIG_TINYDRM_DSI)
> +int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi);
> +#else
> +static inline int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static inline int mipi_dsi_maybe_register_tiny_driver(struct mipi_dsi_device *dsi)
> +{
> +	/* Register the TinyDRM DSI/DBI driver if the panel has no controller */
> +	if (!of_graph_get_port_by_id(dsi->dev.of_node, 0))
> +		return mipi_dsi_register_tiny_driver(dsi);
> +
> +	return 0;
> +}
> +
>   #endif /* __DRM_MIPI_DSI__ */
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Daniel Vetter]

On Tue, Jul 07, 2020 at 04:32:25PM +0200, Noralf Trønnes wrote:
> (cc Dillon)
> 
> Den 03.07.2020 19.26, skrev Sam Ravnborg:
> > Hi Noralf/Paul.
> > 
> > Trying to stir up this discussion again.
> > 
> > On Sun, Jun 14, 2020 at 06:36:22PM +0200, Noralf Trønnes wrote:
> >>
> >>
> >> Den 07.06.2020 15.38, skrev Paul Cercueil:
> >>> Hi,
> >>>
> >>> Here's a follow-up on the previous discussion about the current state of
> >>> DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
> >>>
> >>> This patchset introduces the following:
> >>> * It slightly tweaks the MIPI DSI code so that it supports MIPI DBI over
> >>>   various buses. This patch has been tested with a non-upstream DRM
> >>>   panel driver for a ILI9331 DBI/8080 panel, written with the DSI
> >>>   framework (and doesn't include <drm/drm_mipi_dbi.h>), and non-upstream
> >>>   DSI/DBI host driver for the Ingenic SoCs.
> >>>
> >>> * A SPI DBI host driver, using the current MIPI DSI framework. It allows
> >>>   MIPI DSI/DBI drivers to be written with the DSI framework, even if
> >>>   they are connected over SPI, instead of registering as SPI device
> >>>   drivers. Since most of these panels can be connected over various
> >>>   buses, it permits to reuse the same driver independently of the bus
> >>>   used.
> >>>
> >>> * A TinyDRM driver for DSI/DBI panels, once again independent of the bus
> >>>   used; the only dependency (currently) being that the panel must
> >>>   understand DCS commands.
> >>>
> >>> * A DRM panel driver to test the stack. This driver controls Ilitek
> >>>   ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
> >>>   TFT LCD panel. This panel was converted from
> >>>   drivers/gpu/drm/tiny/ili9341.c.
> >>>
> >>> I would like to emphasize that while it has been compile-tested, I did
> >>> not test it with real hardware since I do not have any DBI panel
> >>> connected over SPI. I did runtime-test the code, just without any panel
> >>> connected.
> >>>
> >>> Another thing to note, is that it does not break Device Tree ABI. The
> >>> display node stays the same:
> >>>
> >>> display@0 {
> >>> 	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
> >>> 	reg = <0>;
> >>> 	spi-max-frequency = <32000000>;
> >>> 	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> >>> 	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> >>> 	rotation = <270>;
> >>> 	backlight = <&backlight>;
> >>> };
> >>>
> >>> The reason it works, is that the "adafruit,yx240qv29" device is probed
> >>> on the SPI bus, so it will match with the SPI/DBI host driver. This will
> >>> in turn register the very same node with the DSI bus, and the ILI9341
> >>> DRM panel driver will probe. The driver will detect that no controller
> >>> is linked to the panel, and eventually register the DBI/DSI TinyDRM
> >>> driver.
> >>>
> >>> I can't stress it enough that this is a RFC, so it still has very rough
> >>> edges.
> >>>
> >>
> >> I don't know bridge and dsi drivers so I can't comment on that, but one
> >> thing I didn't like is that the DT compatible string has to be added to
> >> 2 different modules.
> >>
> >> As an example, a MI0283QT panel (ILI9341) supports these interface options:
> >>
> >> 1. SPI
> >>    Panel setup/control and framebuffer upload over SPI
> >>
> >> 2. SPI + DPI
> >>    Panel setup/control over SPI, framebuffer scanout over DPI
> >>
> >> 3. Parallel bus
> >>    Panel setup/control and framebuffer upload over parallel bus
> > 
> > To continue the configurations we should support:
> > - Panels where the chip can be configured to SPI, SPI+DPI, Parallel bus
> >   (as detailed by Noralf above)
> > - Panels that supports only 6800 or 8080 - connected via GPIO pins or
> >   memory mapped (maybe behind some special IP to support this)
> >   Command set is often special.
> > 
> > We will see a number of chips with many different types of displays.
> > So the drivers should be chip specific with configuration depending on
> > the connected display.
> > 
> > What I hope we can find a solution for is a single file/driver that can
> > support all the relevant interface types for a chip.
> > So we would end up with a single file that included the necessary
> > support for ili9341 in all interface configurations with the necessary
> > support for the relevant displays.
> > 
> > I do not know how far we are from this as I have not dived into the
> > details of any of the proposals.
> 
> In an ideal world I would have liked to see the MIPI DBI parallel
> interface implemented using a new Linux parallel bus type. It could have
> drivers for gpio bitbanging and mmio in addition to other hw specific
> drivers. Now we could have a drm_mipi_dbi DRM driver that registers as a
> SPI client driver and a Parallel bus client driver. Or it can be a
> component driver for the existing DRM driver on the SoC.
> 
> I had plans to do this and made a prototype, but dropped it since it
> would probably require a lot of work getting in a new Linux bus type.

Channelling my inner Greg KH:

Please just create a new bus, it should be quite easy and boilerplate is
manageable.

Greg, did I get this right? Maybe any recommendations for a simple
parallel bus with perhaps different register access paths depending upon
how it's all wired up exactly?
-Daniel

> However if we're going to treat this parallel bus only as a MIPI DBI
> display interface but support gpio bitbanging and mmio as well, then we
> could add DRM drivers for each MIPI DBI bus (that don't have special
> parallel bus hw):
> - mipi-dbi-spi
> - mipi-dbi-gpio
> - mipi-dbi-mmio
> 
> These drivers will register as a mipi_dsi_host adapted like Paul suggested.
> 
> The panel drivers will be mipi_dsi_drivers. Now the panels should work
> regardless of bus type. They probably need to know about the bus type,
> at least whether the parallell bus is 8-bit or 16-bit wide.
> 
> The current MIPI DBI SPI drivers (drm/tiny) will need to be treated
> specially to keep working with old Device Trees when moved over to
> drm/panel.
> 
> Noralf.
> 
> 
> >>
> >> My suggestion is to have one panel driver module that can support all of
> >> these like this:
> > So I think we agree here.
> > 
> >>
> >> For 1. and 2. a SPI driver is registered and if I understand your
> >> example correctly of_graph_get_port_by_id() can be used during probe to
> >> distinguish between the 2 options and register a full DRM driver for 1.
> >> and add a DRM panel for 2.
> >>
> >> For 3. a DSI driver is registered (adapted for DBI use like you're
> >> suggesting).
> >>
> >> Note that the interface part of the controller initialization will
> >> differ between these, the panel side init will be the same I assume.
> > 
> > 	Sam
> > 

-- 
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

Re: [RFC PATCH 3/4] gpu/drm: Add TinyDRM for DSI/DBI panels

[Paul Cercueil]

Hi Sandy,

Le mer. 8 juil. 2020 à 10:26, Sandy Huang <sandy.huang01@yahoo.com> a 
écrit :
> Hi paul,
> 
>     After add this driver, the followinig usage scenarios can be 
> supported?
> 
>     panel 1. crtc->encoder->connector[edp/hdmi/mipi dsi] -> panel
> 
>     panel 2. Panel setup/control and framebuffer upload over SPI
> 
>     the two panel maybe display same/different conctent at same time.

Yes, should be totally possible.

Cheers,
-Paul

> 
> 在 2020/6/7 21:38, Paul Cercueil 写道:
>> The new API function mipi_dsi_maybe_register_tiny_driver() is 
>> supposed
>> to be called by DSI/DBI panel drivers at the end of their probe.
>> 
>> If it is detected that the panel is not connected to any controller,
>> because it has no port #0 node in Device Tree that points back to it,
>> then a TinyDRM driver is registered with it.
>> 
>> This TinyDRM driver expects that a DCS-compliant protocol is used by 
>> the
>> DSI/DBI panel and can only be used with these.
>> 
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>>   drivers/gpu/drm/tiny/Kconfig    |   8 +
>>   drivers/gpu/drm/tiny/Makefile   |   1 +
>>   drivers/gpu/drm/tiny/tiny-dsi.c | 262 
>> ++++++++++++++++++++++++++++++++
>>   include/drm/drm_mipi_dsi.h      |  19 +++
>>   4 files changed, 290 insertions(+)
>>   create mode 100644 drivers/gpu/drm/tiny/tiny-dsi.c
>> 
>> diff --git a/drivers/gpu/drm/tiny/Kconfig 
>> b/drivers/gpu/drm/tiny/Kconfig
>> index 4160e74e4751..54ee58aecf66 100644
>> --- a/drivers/gpu/drm/tiny/Kconfig
>> +++ b/drivers/gpu/drm/tiny/Kconfig
>> @@ -9,6 +9,14 @@ config DRM_GM12U320
>>   	 This is a KMS driver for projectors which use the GM12U320 
>> chipset
>>   	 for video transfer over USB2/3, such as the Acer C120 mini 
>> projector.
>>   \x7f+config TINYDRM_DSI
>> +	tristate "DRM support for generic DBI/DSI display panels"
>> +	depends on DRM && DRM_MIPI_DSI
>> +	select DRM_MIPI_DBI
>> +	select DRM_KMS_CMA_HELPER
>> +	help
>> +	  DRM driver for generic DBI/DSI display panels
>> +
>>   config TINYDRM_HX8357D
>>   	tristate "DRM support for HX8357D display panels"
>>   	depends on DRM && SPI
>> diff --git a/drivers/gpu/drm/tiny/Makefile 
>> b/drivers/gpu/drm/tiny/Makefile
>> index c96ceee71453..49513db9a307 100644
>> --- a/drivers/gpu/drm/tiny/Makefile
>> +++ b/drivers/gpu/drm/tiny/Makefile
>> @@ -1,6 +1,7 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   \x7f  obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
>> +obj-$(CONFIG_TINYDRM_DSI)		+= tiny-dsi.o
>>   obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
>>   obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
>>   obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>> diff --git a/drivers/gpu/drm/tiny/tiny-dsi.c 
>> b/drivers/gpu/drm/tiny/tiny-dsi.c
>> new file mode 100644
>> index 000000000000..915e598844bd
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tiny/tiny-dsi.c
>> @@ -0,0 +1,262 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * TinyDRM driver for standard DSI/DBI panels
>> + *
>> + * Copyright 2020 Paul Cercueil <paul@crapouillou.net>
>> + */
>> +
>> +#include <linux/module.h>
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_damage_helper.h>
>> +#include <drm/drm_drv.h>
>> +#include <drm/drm_fb_helper.h>
>> +#include <drm/drm_fourcc.h>
>> +#include <drm/drm_gem_cma_helper.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
>> +#include <drm/drm_mipi_dbi.h>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_modeset_helper.h>
>> +#include <drm/drm_panel.h>
>> +#include <drm/drm_probe_helper.h>
>> +
>> +#include <video/mipi_display.h>
>> +
>> +struct tiny_dsi {
>> +	struct drm_device drm;
>> +	struct drm_connector connector;
>> +	struct drm_simple_display_pipe pipe;
>> +
>> +	struct mipi_dsi_device *dsi;
>> +	struct drm_panel *panel;
>> +};
>> +
>> +#define mipi_dcs_command(dsi, cmd, seq...) \
>> +({ \
>> +	u8 d[] = { seq }; \
>> +	mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
>> +})
>> +
>> +static inline struct tiny_dsi *drm_to_tiny_dsi(struct drm_device 
>> *drm)
>> +{
>> +	return container_of(drm, struct tiny_dsi, drm);
>> +}
>> +
>> +static void tiny_dsi_fb_dirty(struct drm_framebuffer *fb, struct 
>> drm_rect *rect)
>> +{
>> +	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
>> +	struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem);
>> +	struct tiny_dsi *priv = drm_to_tiny_dsi(fb->dev);
>> +	unsigned int height = rect->y2 - rect->y1;
>> +	unsigned int width = rect->x2 - rect->x1;
>> +	bool fb_convert;
>> +	int idx, ret;
>> +	void *tr;
>> +
>> +	if (!drm_dev_enter(fb->dev, &idx))
>> +		return;
>> +
>> +	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, 
>> DRM_RECT_ARG(rect));
>> +
>> +	fb_convert = width != fb->width || height != fb->height
>> +		|| fb->format->format == DRM_FORMAT_XRGB8888;
>> +	if (fb_convert) {
>> +		tr = kzalloc(width * height * 2, GFP_KERNEL);
>> +
>> +		/* TODO: swap pixels if needed */
>> +		ret = mipi_dbi_buf_copy(tr, fb, rect, false);
>> +		if (ret)
>> +			goto err_msg;
>> +	} else {
>> +		tr = cma_obj->vaddr;
>> +	}
>> +
>> +	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_COLUMN_ADDRESS,
>> +			 (rect->x1 >> 8) & 0xff, rect->x1 & 0xff,
>> +			 (rect->x2 >> 8) & 0xff, rect->x2 & 0xff);
>> +	mipi_dcs_command(priv->dsi, MIPI_DCS_SET_PAGE_ADDRESS,
>> +			 (rect->y1 >> 8) & 0xff, rect->y1 & 0xff,
>> +			 (rect->y2 >> 8) & 0xff, rect->y2 & 0xff);
>> +
>> +	ret = mipi_dsi_dcs_write(priv->dsi, MIPI_DCS_WRITE_MEMORY_START,
>> +				 tr, width * height * 2);
>> +err_msg:
>> +	if (ret)
>> +		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
>> +
>> +	if (fb_convert)
>> +		kfree(tr);
>> +	drm_dev_exit(idx);
>> +}
>> +
>> +static void tiny_dsi_enable(struct drm_simple_display_pipe *pipe,
>> +			    struct drm_crtc_state *crtc_state,
>> +			    struct drm_plane_state *plane_state)
>> +{
>> +	struct tiny_dsi *priv = drm_to_tiny_dsi(pipe->crtc.dev);
>> +
>> +	drm_panel_enable(priv->panel);
>> +}
>> +
>> +static void tiny_dsi_disable(struct drm_simple_display_pipe *pipe)
>> +{
>> +	struct tiny_dsi *priv = drm_to_tiny_dsi(pipe->crtc.dev);
>> +
>> +	drm_panel_disable(priv->panel);
>> +}
>> +
>> +static void tiny_dsi_update(struct drm_simple_display_pipe *pipe,
>> +			    struct drm_plane_state *old_state)
>> +{
>> +	struct drm_plane_state *state = pipe->plane.state;
>> +	struct drm_rect rect;
>> +
>> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
>> +		tiny_dsi_fb_dirty(state->fb, &rect);
>> +}
>> +
>> +static const struct drm_simple_display_pipe_funcs 
>> tiny_dsi_pipe_funcs = {
>> +	.enable = tiny_dsi_enable,
>> +	.disable = tiny_dsi_disable,
>> +	.update = tiny_dsi_update,
>> +	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +};
>> +
>> +static int tiny_dsi_connector_get_modes(struct drm_connector 
>> *connector)
>> +{
>> +	struct tiny_dsi *priv = drm_to_tiny_dsi(connector->dev);
>> +
>> +	return drm_panel_get_modes(priv->panel, connector);
>> +}
>> +
>> +static const struct drm_connector_helper_funcs 
>> tiny_dsi_connector_hfuncs = {
>> +	.get_modes = tiny_dsi_connector_get_modes,
>> +};
>> +
>> +static const struct drm_connector_funcs tiny_dsi_connector_funcs = {
>> +	.reset = drm_atomic_helper_connector_reset,
>> +	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.destroy = drm_connector_cleanup,
>> +	.atomic_duplicate_state = 
>> drm_atomic_helper_connector_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +DEFINE_DRM_GEM_CMA_FOPS(tiny_dsi_fops);
>> +
>> +static const uint32_t tiny_dsi_formats[] = {
>> +	DRM_FORMAT_RGB565,
>> +	DRM_FORMAT_XRGB8888,
>> +};
>> +
>> +static const struct drm_mode_config_funcs 
>> tiny_dsi_mode_config_funcs = {
>> +	.fb_create = drm_gem_fb_create_with_dirty,
>> +	.atomic_check = drm_atomic_helper_check,
>> +	.atomic_commit = drm_atomic_helper_commit,
>> +};
>> +
>> +static void tiny_dsi_release(struct drm_device *drm)
>> +{
>> +	struct tiny_dsi *priv = drm_to_tiny_dsi(drm);
>> +
>> +	drm_mode_config_cleanup(drm);
>> +	drm_dev_fini(drm);
>> +	kfree(priv);
>> +}
>> +
>> +static struct drm_driver tiny_dsi_driver = {
>> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>> +	.fops			= &tiny_dsi_fops,
>> +	.release		= tiny_dsi_release,
>> +	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	.name			= "tiny-dsi",
>> +	.desc			= "Tiny DSI",
>> +	.date			= "20200605",
>> +	.major			= 1,
>> +	.minor			= 0,
>> +};
>> +
>> +static void tiny_dsi_remove(void *drm)
>> +{
>> +	drm_dev_unplug(drm);
>> +	drm_atomic_helper_shutdown(drm);
>> +}
>> +
>> +int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi)
>> +{
>> +	struct device *dev = &dsi->dev;
>> +	struct drm_device *drm;
>> +	struct tiny_dsi *priv;
>> +	static const uint64_t modifiers[] = {
>> +		DRM_FORMAT_MOD_LINEAR,
>> +		DRM_FORMAT_MOD_INVALID
>> +	};
>> +	int ret;
>> +
>> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->dsi = dsi;
>> +	drm = &priv->drm;
>> +
>> +	ret = devm_drm_dev_init(dev, drm, &tiny_dsi_driver);
>> +	if (ret) {
>> +		kfree(priv);
>> +		return ret;
>> +	}
>> +
>> +	drm_mode_config_init(drm);
>> +
>> +	priv->panel = of_drm_find_panel(dev->of_node);
>> +	if (IS_ERR(priv->panel)) {
>> +		dev_err(dev, "Unable to find panel\n");
>> +		return PTR_ERR(priv->panel);
>> +	}
>> +
>> +	drm->mode_config.preferred_depth = 16;
>> +
>> +	drm->mode_config.funcs = &tiny_dsi_mode_config_funcs;
>> +	drm->mode_config.min_width = 0;
>> +	drm->mode_config.min_height = 0;
>> +	drm->mode_config.max_width = 4096;
>> +	drm->mode_config.max_height = 4096;
>> +
>> +	drm_connector_helper_add(&priv->connector, 
>> &tiny_dsi_connector_hfuncs);
>> +	ret = drm_connector_init(drm, &priv->connector, 
>> &tiny_dsi_connector_funcs,
>> +				 DRM_MODE_CONNECTOR_DSI);
>> +	if (ret) {
>> +		dev_err(dev, "Unable to init connector\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = drm_simple_display_pipe_init(drm, &priv->pipe, 
>> &tiny_dsi_pipe_funcs,
>> +					   tiny_dsi_formats, ARRAY_SIZE(tiny_dsi_formats),
>> +					   modifiers, &priv->connector);
>> +	if (ret) {
>> +		dev_err(dev, "Unable to init display pipe\n");
>> +		return ret;
>> +	}
>> +
>> +	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
>> +
>> +	drm_mode_config_reset(drm);
>> +
>> +	ret = drm_dev_register(drm, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to register DRM driver\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = devm_add_action_or_reset(dev, tiny_dsi_remove, drm);
>> +	if (ret)
>> +		return ret;
>> +
>> +	drm_fbdev_generic_setup(drm, 0);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(mipi_dsi_register_tiny_driver);
>> +
>> +MODULE_DESCRIPTION("DSI/DBI TinyDRM driver");
>> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index 65d2961fc054..0c2589a55df6 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -10,6 +10,7 @@
>>   #define __DRM_MIPI_DSI_H__
>>   \x7f  #include <linux/device.h>
>> +#include <linux/of_graph.h>
>>   \x7f  struct mipi_dsi_host;
>>   struct mipi_dsi_device;
>> @@ -337,4 +338,22 @@ void mipi_dsi_driver_unregister(struct 
>> mipi_dsi_driver *driver);
>>   	module_driver(__mipi_dsi_driver, mipi_dsi_driver_register, \
>>   			mipi_dsi_driver_unregister)
>>   \x7f+#if IS_ENABLED(CONFIG_TINYDRM_DSI)
>> +int mipi_dsi_register_tiny_driver(struct mipi_dsi_device *dsi);
>> +#else
>> +static inline int mipi_dsi_register_tiny_driver(struct 
>> mipi_dsi_device *dsi)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static inline int mipi_dsi_maybe_register_tiny_driver(struct 
>> mipi_dsi_device *dsi)
>> +{
>> +	/* Register the TinyDRM DSI/DBI driver if the panel has no 
>> controller */
>> +	if (!of_graph_get_port_by_id(dsi->dev.of_node, 0))
>> +		return mipi_dsi_register_tiny_driver(dsi);
>> +
>> +	return 0;
>> +}
>> +
>>   #endif /* __DRM_MIPI_DSI__ */


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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Paul Cercueil]

Hi Daniel,

Le mer. 8 juil. 2020 à 9:23, Daniel Vetter <daniel@ffwll.ch> a écrit :
> On Tue, Jul 07, 2020 at 04:32:25PM +0200, Noralf Trønnes wrote:
>>  (cc Dillon)
>> 
>>  Den 03.07.2020 19.26, skrev Sam Ravnborg:
>>  > Hi Noralf/Paul.
>>  >
>>  > Trying to stir up this discussion again.
>>  >
>>  > On Sun, Jun 14, 2020 at 06:36:22PM +0200, Noralf Trønnes wrote:
>>  >>
>>  >>
>>  >> Den 07.06.2020 15.38, skrev Paul Cercueil:
>>  >>> Hi,
>>  >>>
>>  >>> Here's a follow-up on the previous discussion about the current 
>> state of
>>  >>> DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
>>  >>>
>>  >>> This patchset introduces the following:
>>  >>> * It slightly tweaks the MIPI DSI code so that it supports MIPI 
>> DBI over
>>  >>>   various buses. This patch has been tested with a non-upstream 
>> DRM
>>  >>>   panel driver for a ILI9331 DBI/8080 panel, written with the 
>> DSI
>>  >>>   framework (and doesn't include <drm/drm_mipi_dbi.h>), and 
>> non-upstream
>>  >>>   DSI/DBI host driver for the Ingenic SoCs.
>>  >>>
>>  >>> * A SPI DBI host driver, using the current MIPI DSI framework. 
>> It allows
>>  >>>   MIPI DSI/DBI drivers to be written with the DSI framework, 
>> even if
>>  >>>   they are connected over SPI, instead of registering as SPI 
>> device
>>  >>>   drivers. Since most of these panels can be connected over 
>> various
>>  >>>   buses, it permits to reuse the same driver independently of 
>> the bus
>>  >>>   used.
>>  >>>
>>  >>> * A TinyDRM driver for DSI/DBI panels, once again independent 
>> of the bus
>>  >>>   used; the only dependency (currently) being that the panel 
>> must
>>  >>>   understand DCS commands.
>>  >>>
>>  >>> * A DRM panel driver to test the stack. This driver controls 
>> Ilitek
>>  >>>   ILI9341 based DBI panels, like the Adafruit YX240QV29-T 
>> 320x240 2.4"
>>  >>>   TFT LCD panel. This panel was converted from
>>  >>>   drivers/gpu/drm/tiny/ili9341.c.
>>  >>>
>>  >>> I would like to emphasize that while it has been 
>> compile-tested, I did
>>  >>> not test it with real hardware since I do not have any DBI panel
>>  >>> connected over SPI. I did runtime-test the code, just without 
>> any panel
>>  >>> connected.
>>  >>>
>>  >>> Another thing to note, is that it does not break Device Tree 
>> ABI. The
>>  >>> display node stays the same:
>>  >>>
>>  >>> display@0 {
>>  >>> 	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
>>  >>> 	reg = <0>;
>>  >>> 	spi-max-frequency = <32000000>;
>>  >>> 	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>>  >>> 	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>>  >>> 	rotation = <270>;
>>  >>> 	backlight = <&backlight>;
>>  >>> };
>>  >>>
>>  >>> The reason it works, is that the "adafruit,yx240qv29" device is 
>> probed
>>  >>> on the SPI bus, so it will match with the SPI/DBI host driver. 
>> This will
>>  >>> in turn register the very same node with the DSI bus, and the 
>> ILI9341
>>  >>> DRM panel driver will probe. The driver will detect that no 
>> controller
>>  >>> is linked to the panel, and eventually register the DBI/DSI 
>> TinyDRM
>>  >>> driver.
>>  >>>
>>  >>> I can't stress it enough that this is a RFC, so it still has 
>> very rough
>>  >>> edges.
>>  >>>
>>  >>
>>  >> I don't know bridge and dsi drivers so I can't comment on that, 
>> but one
>>  >> thing I didn't like is that the DT compatible string has to be 
>> added to
>>  >> 2 different modules.
>>  >>
>>  >> As an example, a MI0283QT panel (ILI9341) supports these 
>> interface options:
>>  >>
>>  >> 1. SPI
>>  >>    Panel setup/control and framebuffer upload over SPI
>>  >>
>>  >> 2. SPI + DPI
>>  >>    Panel setup/control over SPI, framebuffer scanout over DPI
>>  >>
>>  >> 3. Parallel bus
>>  >>    Panel setup/control and framebuffer upload over parallel bus
>>  >
>>  > To continue the configurations we should support:
>>  > - Panels where the chip can be configured to SPI, SPI+DPI, 
>> Parallel bus
>>  >   (as detailed by Noralf above)
>>  > - Panels that supports only 6800 or 8080 - connected via GPIO 
>> pins or
>>  >   memory mapped (maybe behind some special IP to support this)
>>  >   Command set is often special.
>>  >
>>  > We will see a number of chips with many different types of 
>> displays.
>>  > So the drivers should be chip specific with configuration 
>> depending on
>>  > the connected display.
>>  >
>>  > What I hope we can find a solution for is a single file/driver 
>> that can
>>  > support all the relevant interface types for a chip.
>>  > So we would end up with a single file that included the necessary
>>  > support for ili9341 in all interface configurations with the 
>> necessary
>>  > support for the relevant displays.
>>  >
>>  > I do not know how far we are from this as I have not dived into 
>> the
>>  > details of any of the proposals.
>> 
>>  In an ideal world I would have liked to see the MIPI DBI parallel
>>  interface implemented using a new Linux parallel bus type. It could 
>> have
>>  drivers for gpio bitbanging and mmio in addition to other hw 
>> specific
>>  drivers. Now we could have a drm_mipi_dbi DRM driver that registers 
>> as a
>>  SPI client driver and a Parallel bus client driver. Or it can be a
>>  component driver for the existing DRM driver on the SoC.
>> 
>>  I had plans to do this and made a prototype, but dropped it since it
>>  would probably require a lot of work getting in a new Linux bus 
>> type.
> 
> Channelling my inner Greg KH:
> 
> Please just create a new bus, it should be quite easy and boilerplate 
> is
> manageable.

The bus is already here, it's "mipi-dsi". DBI and DSI are basically the 
same thing, just that one is parallel and the other is serial.

-Paul

> Greg, did I get this right? Maybe any recommendations for a simple
> parallel bus with perhaps different register access paths depending 
> upon
> how it's all wired up exactly?
> -Daniel
> 
>>  However if we're going to treat this parallel bus only as a MIPI DBI
>>  display interface but support gpio bitbanging and mmio as well, 
>> then we
>>  could add DRM drivers for each MIPI DBI bus (that don't have special
>>  parallel bus hw):
>>  - mipi-dbi-spi
>>  - mipi-dbi-gpio
>>  - mipi-dbi-mmio
>> 
>>  These drivers will register as a mipi_dsi_host adapted like Paul 
>> suggested.
>> 
>>  The panel drivers will be mipi_dsi_drivers. Now the panels should 
>> work
>>  regardless of bus type. They probably need to know about the bus 
>> type,
>>  at least whether the parallell bus is 8-bit or 16-bit wide.
>> 
>>  The current MIPI DBI SPI drivers (drm/tiny) will need to be treated
>>  specially to keep working with old Device Trees when moved over to
>>  drm/panel.
>> 
>>  Noralf.
>> 
>> 
>>  >>
>>  >> My suggestion is to have one panel driver module that can 
>> support all of
>>  >> these like this:
>>  > So I think we agree here.
>>  >
>>  >>
>>  >> For 1. and 2. a SPI driver is registered and if I understand your
>>  >> example correctly of_graph_get_port_by_id() can be used during 
>> probe to
>>  >> distinguish between the 2 options and register a full DRM driver 
>> for 1.
>>  >> and add a DRM panel for 2.
>>  >>
>>  >> For 3. a DSI driver is registered (adapted for DBI use like 
>> you're
>>  >> suggesting).
>>  >>
>>  >> Note that the interface part of the controller initialization 
>> will
>>  >> differ between these, the panel side init will be the same I 
>> assume.
>>  >
>>  > 	Sam
>>  >
> 
> --
> 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

Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Noralf Trønnes]

Den 08.07.2020 14.49, skrev Paul Cercueil:
> Hi Daniel,
> 
> Le mer. 8 juil. 2020 à 9:23, Daniel Vetter <daniel@ffwll.ch> a écrit :
>> On Tue, Jul 07, 2020 at 04:32:25PM +0200, Noralf Trønnes wrote:
>>>  (cc Dillon)
>>>
>>>  Den 03.07.2020 19.26, skrev Sam Ravnborg:
>>>  > Hi Noralf/Paul.
>>>  >
>>>  > Trying to stir up this discussion again.
>>>  >
>>>  > On Sun, Jun 14, 2020 at 06:36:22PM +0200, Noralf Trønnes wrote:
>>>  >>
>>>  >>
>>>  >> Den 07.06.2020 15.38, skrev Paul Cercueil:
>>>  >>> Hi,
>>>  >>>
>>>  >>> Here's a follow-up on the previous discussion about the current
>>> state of
>>>  >>> DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
>>>  >>>
>>>  >>> This patchset introduces the following:
>>>  >>> * It slightly tweaks the MIPI DSI code so that it supports MIPI
>>> DBI over
>>>  >>>   various buses. This patch has been tested with a non-upstream DRM
>>>  >>>   panel driver for a ILI9331 DBI/8080 panel, written with the DSI
>>>  >>>   framework (and doesn't include <drm/drm_mipi_dbi.h>), and
>>> non-upstream
>>>  >>>   DSI/DBI host driver for the Ingenic SoCs.
>>>  >>>
>>>  >>> * A SPI DBI host driver, using the current MIPI DSI framework.
>>> It allows
>>>  >>>   MIPI DSI/DBI drivers to be written with the DSI framework,
>>> even if
>>>  >>>   they are connected over SPI, instead of registering as SPI device
>>>  >>>   drivers. Since most of these panels can be connected over various
>>>  >>>   buses, it permits to reuse the same driver independently of
>>> the bus
>>>  >>>   used.
>>>  >>>
>>>  >>> * A TinyDRM driver for DSI/DBI panels, once again independent of
>>> the bus
>>>  >>>   used; the only dependency (currently) being that the panel must
>>>  >>>   understand DCS commands.
>>>  >>>
>>>  >>> * A DRM panel driver to test the stack. This driver controls Ilitek
>>>  >>>   ILI9341 based DBI panels, like the Adafruit YX240QV29-T
>>> 320x240 2.4"
>>>  >>>   TFT LCD panel. This panel was converted from
>>>  >>>   drivers/gpu/drm/tiny/ili9341.c.
>>>  >>>
>>>  >>> I would like to emphasize that while it has been compile-tested,
>>> I did
>>>  >>> not test it with real hardware since I do not have any DBI panel
>>>  >>> connected over SPI. I did runtime-test the code, just without
>>> any panel
>>>  >>> connected.
>>>  >>>
>>>  >>> Another thing to note, is that it does not break Device Tree
>>> ABI. The
>>>  >>> display node stays the same:
>>>  >>>
>>>  >>> display@0 {
>>>  >>>     compatible = "adafruit,yx240qv29", "ilitek,ili9341";
>>>  >>>     reg = <0>;
>>>  >>>     spi-max-frequency = <32000000>;
>>>  >>>     dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
>>>  >>>     reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>>>  >>>     rotation = <270>;
>>>  >>>     backlight = <&backlight>;
>>>  >>> };
>>>  >>>
>>>  >>> The reason it works, is that the "adafruit,yx240qv29" device is
>>> probed
>>>  >>> on the SPI bus, so it will match with the SPI/DBI host driver.
>>> This will
>>>  >>> in turn register the very same node with the DSI bus, and the
>>> ILI9341
>>>  >>> DRM panel driver will probe. The driver will detect that no
>>> controller
>>>  >>> is linked to the panel, and eventually register the DBI/DSI TinyDRM
>>>  >>> driver.
>>>  >>>
>>>  >>> I can't stress it enough that this is a RFC, so it still has
>>> very rough
>>>  >>> edges.
>>>  >>>
>>>  >>
>>>  >> I don't know bridge and dsi drivers so I can't comment on that,
>>> but one
>>>  >> thing I didn't like is that the DT compatible string has to be
>>> added to
>>>  >> 2 different modules.
>>>  >>
>>>  >> As an example, a MI0283QT panel (ILI9341) supports these
>>> interface options:
>>>  >>
>>>  >> 1. SPI
>>>  >>    Panel setup/control and framebuffer upload over SPI
>>>  >>
>>>  >> 2. SPI + DPI
>>>  >>    Panel setup/control over SPI, framebuffer scanout over DPI
>>>  >>
>>>  >> 3. Parallel bus
>>>  >>    Panel setup/control and framebuffer upload over parallel bus
>>>  >
>>>  > To continue the configurations we should support:
>>>  > - Panels where the chip can be configured to SPI, SPI+DPI,
>>> Parallel bus
>>>  >   (as detailed by Noralf above)
>>>  > - Panels that supports only 6800 or 8080 - connected via GPIO pins or
>>>  >   memory mapped (maybe behind some special IP to support this)
>>>  >   Command set is often special.
>>>  >
>>>  > We will see a number of chips with many different types of displays.
>>>  > So the drivers should be chip specific with configuration
>>> depending on
>>>  > the connected display.
>>>  >
>>>  > What I hope we can find a solution for is a single file/driver
>>> that can
>>>  > support all the relevant interface types for a chip.
>>>  > So we would end up with a single file that included the necessary
>>>  > support for ili9341 in all interface configurations with the
>>> necessary
>>>  > support for the relevant displays.
>>>  >
>>>  > I do not know how far we are from this as I have not dived into the
>>>  > details of any of the proposals.
>>>
>>>  In an ideal world I would have liked to see the MIPI DBI parallel
>>>  interface implemented using a new Linux parallel bus type. It could
>>> have
>>>  drivers for gpio bitbanging and mmio in addition to other hw specific
>>>  drivers. Now we could have a drm_mipi_dbi DRM driver that registers
>>> as a
>>>  SPI client driver and a Parallel bus client driver. Or it can be a
>>>  component driver for the existing DRM driver on the SoC.

I have realised that there is a problem here. A drm_mipi_drm driver
would require a device and this device would be virtual since the actual
device is the panel itself. IIRC we can't have virtual devices in the
Device Tree. So this won't work I think.
The same problem applies to my mipi-dbi-spi case below, it is also a
virtual device.

>>>
>>>  I had plans to do this and made a prototype, but dropped it since it
>>>  would probably require a lot of work getting in a new Linux bus type.
>>
>> Channelling my inner Greg KH:
>>
>> Please just create a new bus, it should be quite easy and boilerplate is
>> manageable.

I'm not sure I agree with that. Wouldn't this be something akin to SPI
or I2C? There's quite some code in those subsystems.

> 
> The bus is already here, it's "mipi-dsi". DBI and DSI are basically the
> same thing, just that one is parallel and the other is serial.

Can the parallel interface on your hw be used outside the display domain
as say connected to an ADC? If not then a new Linux bus type won't help
in your case.

My concern is tying a parallel interface to the DRM subsystem if it can
be used as a generic interface. The Raspberry Pi has a SMI (Secondary
Memory Interface) parallel interface that can be used for DBI and I
believe the Beaglebone Black can have a hw bus through the realtime
unit. But I have no experience with either of these.

Noralf.

> 
> -Paul
> 
>> Greg, did I get this right? Maybe any recommendations for a simple
>> parallel bus with perhaps different register access paths depending upon
>> how it's all wired up exactly?
>> -Daniel
>>
>>>  However if we're going to treat this parallel bus only as a MIPI DBI
>>>  display interface but support gpio bitbanging and mmio as well, then we
>>>  could add DRM drivers for each MIPI DBI bus (that don't have special
>>>  parallel bus hw):
>>>  - mipi-dbi-spi
>>>  - mipi-dbi-gpio
>>>  - mipi-dbi-mmio
>>>
>>>  These drivers will register as a mipi_dsi_host adapted like Paul
>>> suggested.
>>>
>>>  The panel drivers will be mipi_dsi_drivers. Now the panels should work
>>>  regardless of bus type. They probably need to know about the bus type,
>>>  at least whether the parallell bus is 8-bit or 16-bit wide.
>>>
>>>  The current MIPI DBI SPI drivers (drm/tiny) will need to be treated
>>>  specially to keep working with old Device Trees when moved over to
>>>  drm/panel.
>>>
>>>  Noralf.
>>>
>>>
>>>  >>
>>>  >> My suggestion is to have one panel driver module that can support
>>> all of
>>>  >> these like this:
>>>  > So I think we agree here.
>>>  >
>>>  >>
>>>  >> For 1. and 2. a SPI driver is registered and if I understand your
>>>  >> example correctly of_graph_get_port_by_id() can be used during
>>> probe to
>>>  >> distinguish between the 2 options and register a full DRM driver
>>> for 1.
>>>  >> and add a DRM panel for 2.
>>>  >>
>>>  >> For 3. a DSI driver is registered (adapted for DBI use like you're
>>>  >> suggesting).
>>>  >>
>>>  >> Note that the interface part of the controller initialization will
>>>  >> differ between these, the panel side init will be the same I assume.
>>>  >
>>>  >     Sam
>>>  >
>>
>> -- 
>> 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