[PATCH v2 0/6] DSI/DBI, panel drivers, & tinyDRM v2

[PATCH v2 0/6] DSI/DBI, panel drivers, & tinyDRM v2

[Paul Cercueil]

Hi,

Here's a V2 of my patchset that attempts to clean up the current
situation with DSI/DBI panels drivers, and tinyDRM.

For the record, here is a small sum-up of the current situation:

- the current MIPI DBI code (drivers/gpu/drm/drm_mipi_dbi.c) is lagging
  way behind the MIPI DSI code (drivers/gpu/drm/drm_mipi_dsi.c). While
  the DSI code adds a proper bus support, with support for host drivers
  and client devices, there is no such thing with the DBI code. As such,
  it is currently impossible to write a standard DRM panel driver for a
  DBI panel.

- Even if the MIPI DBI code was updated with a proper bus, many panels
  and MIPI controllers support both DSI and DBI, so it would be a pain
  to support them without resolving to duplicating each driver.

- The panel drivers written against the DBI code are all "tinyDRM"
  drivers, which means that they will register a full yet simple DRM
  driver, and cannot be used as regular DRM panels for a different DRM
  driver.

- These "tinyDRM" drivers all use SPI directly, even though the panels
  they're driving can work on other interfaces (e.g. i8080 bus). Which
  means that one driver written for e.g. a ILI9341 would not work if
  the control interface is not SPI.

- The "tinyDRM" common code is entangled with DBI and there is no clear
  separation between the two. It could very well be moved to a single
  "tinyDRM" driver that works with a DRM panel obtained from devicetree,
  because the only requirement is that the panel supports a few given
  DCS commands.

Noteworthy changes since V1:

* The DT binding document for the NV3052C panel has been updated with
  the feedback I got from V1. It now supports multiple power supplies.

* Instead of using macros to define bus types, we now have an enum
  mipi_dcs_bus_type.

* The WARN_ONE_ONCE() that were in place to check that the host and
  client drivers provided the DCS bus bitmask is gone, we just default
  to DSI instead.

* DBI/SPI driver code was moved out of drivers/gpu/drm/bridge/.

* The DBI/SPI driver is registered as a driver by each client if needed,
  they just call module_mpi_dbi_spi_driver(). This addresses the issue
  in V1 that compatible strings had to be added to two different places.

* NV3052C and ILI9341 panel drivers were updated to remove custom
  backlight handling, call drm_panel_{disable,unprepare} on module exit,
  and various small fixes.

For a more detailed changelog, see the header of each individual patch.

Paul Cercueil (6):
  dt-bindings: display: Document NewVision NV3052C DT node
  drm: dsi: Let host and device specify supported bus
  drm: Add SPI DBI host driver
  drm/tiny: Add TinyDRM for DSI/DBI panels
  drm/panel: Add panel driver for NewVision NV3052C based LCDs
  drm/panel: Add Ilitek ILI9341 DBI panel driver

 .../display/panel/newvision,nv3052c.yaml      | 100 ++++
 drivers/gpu/drm/Kconfig                       |   8 +
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/drm_mipi_dbi_spi.c            | 247 +++++++++
 drivers/gpu/drm/drm_mipi_dsi.c                |   9 +
 drivers/gpu/drm/panel/Kconfig                 |  18 +
 drivers/gpu/drm/panel/Makefile                |   2 +
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c  | 318 +++++++++++
 .../gpu/drm/panel/panel-newvision-nv3052c.c   | 510 ++++++++++++++++++
 drivers/gpu/drm/tiny/Kconfig                  |   8 +
 drivers/gpu/drm/tiny/Makefile                 |   1 +
 drivers/gpu/drm/tiny/tiny-dsi.c               | 266 +++++++++
 include/drm/drm_mipi_dbi_spi.h                |  42 ++
 include/drm/drm_mipi_dsi.h                    |  44 ++
 14 files changed, 1574 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
 create mode 100644 drivers/gpu/drm/drm_mipi_dbi_spi.c
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c
 create mode 100644 drivers/gpu/drm/panel/panel-newvision-nv3052c.c
 create mode 100644 drivers/gpu/drm/tiny/tiny-dsi.c
 create mode 100644 include/drm/drm_mipi_dbi_spi.h

-- 
2.28.0

[PATCH v2 1/6] dt-bindings: display: Document NewVision NV3052C DT node

[Paul Cercueil]

Add documentation for the Device Tree node for LCD panels based on the
NewVision NV3052C controller.

v2: - Support backlight property
    - Add *-supply properties for the 5 different power supplies.
      Either they must all be present, or 'power-supply' must be
      present.
    - Reword description to avoid confusion about 'driver'
    - Use 4-space indent in example

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 .../display/panel/newvision,nv3052c.yaml      | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml b/Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
new file mode 100644
index 000000000000..0468ddeaff2d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/newvision,nv3052c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NewVision NV3052C TFT LCD panel driver with SPI control bus
+
+maintainers:
+  - Paul Cercueil <paul@crapouillou.net>
+
+description: |
+  This is a IC driver for TFT panels, accepting a variety of input
+  streams that get adapted and scaled to the panel.
+
+  The panel must obey the rules for a SPI slave device as specified in
+  spi/spi-controller.yaml
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+        - leadtek,ltk035c5444t-spi
+
+      - const: newvision,nv3052c
+
+  reg:
+    maxItems: 1
+
+  reset-gpios: true
+  power-supply: true
+  backlight: true
+  port: true
+
+  vci-supply:
+    description:
+      Power supply for analog circuits (VCI=2.5V to 6V)
+
+  vddam-supply:
+    description:
+      Power Supply for MIPI regulator circuits (VDDAM=1.75V to 6V)
+
+  iovcc-supply:
+    description: |
+      External Power Supply for IO pads and other logic circuits
+      (IOVCC=1.65 to 3.6V)
+
+  pprech-supply:
+    description:
+      Pre-charge power for source (can be connected to IOVCC or VCI)
+
+  vpp-supply:
+    description:
+      Input power for NV memory programming (8.0V ~ 8.5V, typical=8.25V)
+
+required:
+  - compatible
+  - reg
+
+oneOf:
+  - required:
+    - power-supply
+  - required:
+    - vci-supply
+    - vddam-supply
+    - iovcc-supply
+    - pprech-supply
+    - vpp-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        display@0 {
+            compatible = "leadtek,ltk035c5444t-spi", "newvision,nv3052c";
+            reg = <0>;
+
+            spi-max-frequency = <15000000>;
+            spi-3wire;
+            reset-gpios = <&gpe 2 GPIO_ACTIVE_LOW>;
+            backlight = <&backlight>;
+            power-supply = <&vcc>;
+
+            port {
+                panel_input: endpoint {
+                    remote-endpoint = <&panel_output>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.28.0

Re: [PATCH v2 1/6] dt-bindings: display: Document NewVision NV3052C DT node

[Rob Herring]

On Sat, 22 Aug 2020 18:32:45 +0200, Paul Cercueil wrote:
> Add documentation for the Device Tree node for LCD panels based on the
> NewVision NV3052C controller.
> 
> v2: - Support backlight property
>     - Add *-supply properties for the 5 different power supplies.
>       Either they must all be present, or 'power-supply' must be
>       present.
>     - Reword description to avoid confusion about 'driver'
>     - Use 4-space indent in example
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  .../display/panel/newvision,nv3052c.yaml      | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 1/6] dt-bindings: display: Document NewVision NV3052C DT node

[Linus Walleij]

On Sat, Aug 22, 2020 at 6:33 PM Paul Cercueil <paul@crapouillou.net> wrote:

> Add documentation for the Device Tree node for LCD panels based on the
> NewVision NV3052C controller.
>
> v2: - Support backlight property
>     - Add *-supply properties for the 5 different power supplies.
>       Either they must all be present, or 'power-supply' must be
>       present.
>     - Reword description to avoid confusion about 'driver'
>     - Use 4-space indent in example
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

> +            reset-gpios = <&gpe 2 GPIO_ACTIVE_LOW>;

This has the right polarity but the code in patch 6
seems to use a device tree that does not?

Yours,
Linus Walleij

[PATCH v2 2/6] 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.

v2: - Remove the WARN_ON_ONCE() if (dbi->bus_types == 0), because it
      will trigger for every panel out there. Just default to
      MIPI_DCS_BUS_TYPE_DSI if the bitmask is not populated.
    - Create a 'enum mipi_dcs_bus_type' instead of macros
    - Rename values to avoid confusion about SPI modes

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

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 5dd475e82995..a3cbea8019cc 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -281,6 +281,9 @@ int mipi_dsi_host_register(struct mipi_dsi_host *host)
 {
 	struct device_node *node;
 
+	if (!host->bus_types)
+		host->bus_types = MIPI_DCS_BUS_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))
@@ -323,6 +326,12 @@ int mipi_dsi_attach(struct mipi_dsi_device *dsi)
 {
 	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
 
+	if (!dsi->bus_type)
+		dsi->bus_type = MIPI_DCS_BUS_TYPE_DSI;
+
+	if (!(dsi->bus_type & dsi->host->bus_types))
+		return -EINVAL;
+
 	if (!ops || !ops->attach)
 		return -ENOSYS;
 
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 360e6377e84b..802644c4c0c4 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -63,6 +63,27 @@ struct mipi_dsi_packet {
 int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
 			   const struct mipi_dsi_msg *msg);
 
+/**
+ * enum mipi_dcs_bus_type - MIPI DCS bus types
+ * @MIPI_DCS_BUS_TYPE_DSI: MIPI DSI
+ * @MIPI_DCS_BUS_TYPE_DBI_SPI_C1: DBI with SPI carrier, 9 bits per word, with
+ *    the data/command information in the 9th (MSB) bit
+ * @MIPI_DCS_BUS_TYPE_DBI_SPI_C2: DBI with SPI carrier, 16 bits per word, with
+ *    the data/command information in the 9th bit, and 7 MSB bits of padding
+ * @MIPI_DCS_BUS_TYPE_DBI_SPI_C3: DBI with SPI carrier, 8 bits per word, with
+ *    the data/command information carried by a separate GPIO
+ * @MIPI_DCS_BUS_TYPE_DBI_M6800: Motorola 6800 type parallel bus
+ * @MIPI_DCS_BUS_TYPE_DBI_I8080: Intel 8080 type parallel bus
+ */
+enum mipi_dcs_bus_type {
+	MIPI_DCS_BUS_TYPE_DSI		= BIT(0),
+	MIPI_DCS_BUS_TYPE_DBI_SPI_C1	= BIT(1),
+	MIPI_DCS_BUS_TYPE_DBI_SPI_C2	= BIT(2),
+	MIPI_DCS_BUS_TYPE_DBI_SPI_C3	= BIT(3),
+	MIPI_DCS_BUS_TYPE_DBI_M6800	= BIT(4),
+	MIPI_DCS_BUS_TYPE_DBI_I8080	= BIT(5),
+};
+
 /**
  * struct mipi_dsi_host_ops - DSI bus operations
  * @attach: attach DSI device to DSI host
@@ -94,11 +115,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 (enum mipi_dcs_bus_type)
  * @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 +185,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
  * @channel: virtual channel assigned to the peripheral
  * @format: pixel format for video mode
  * @lanes: number of active data lanes
@@ -178,6 +202,7 @@ struct mipi_dsi_device {
 	struct device dev;
 
 	char name[DSI_DEV_NAME_SIZE];
+	enum mipi_dcs_bus_type bus_type;
 	unsigned int channel;
 	unsigned int lanes;
 	enum mipi_dsi_pixel_format format;
-- 
2.28.0

[PATCH v2 3/6] drm: Add SPI DBI host driver

[Paul Cercueil]

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

DBI types c1 and c3 are supported. C1 is a SPI protocol with 9 bits per
word, with the data/command information in the 9th (MSB) bit. C3 is a
SPI protocol with 8 bits per word, with the data/command information
carried by a separate GPIO.

v2: - Move ouside of drivers/gpu/drm/bridge/
    - The client drivers should now use module_mipi_dbi_spi_driver().
      This ensures that the panel drivers can probe from the DSI/DBI
      bus, as well as from the SPI bus, using a shared OF match table.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/Kconfig            |   8 +
 drivers/gpu/drm/Makefile           |   1 +
 drivers/gpu/drm/drm_mipi_dbi_spi.c | 247 +++++++++++++++++++++++++++++
 include/drm/drm_mipi_dbi_spi.h     |  42 +++++
 4 files changed, 298 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_mipi_dbi_spi.c
 create mode 100644 include/drm/drm_mipi_dbi_spi.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 147d61b9674e..932c7bcaeff0 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -32,6 +32,14 @@ config DRM_MIPI_DSI
 	bool
 	depends on DRM
 
+config DRM_MIPI_DBI_SPI
+	tristate "SPI host support for MIPI DBI"
+	depends on DRM && OF && SPI
+	select DRM_MIPI_DSI
+	select DRM_MIPI_DBI
+	help
+	  Enable this option to support DBI panels connected over SPI.
+
 config DRM_DP_AUX_CHARDEV
 	bool "DRM DP AUX Interface"
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 2f31579f91d4..8c60448a3ee5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
 obj-$(CONFIG_DRM)	+= drm.o
 obj-$(CONFIG_DRM_MIPI_DBI) += drm_mipi_dbi.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
+obj-$(CONFIG_DRM_MIPI_DBI_SPI) += drm_mipi_dbi_spi.o
 obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
 obj-y			+= arm/
 obj-$(CONFIG_DRM_TTM)	+= ttm/
diff --git a/drivers/gpu/drm/drm_mipi_dbi_spi.c b/drivers/gpu/drm/drm_mipi_dbi_spi.c
new file mode 100644
index 000000000000..a7eaf3125a17
--- /dev/null
+++ b/drivers/gpu/drm/drm_mipi_dbi_spi.c
@@ -0,0 +1,247 @@
+// 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;
+
+	unsigned int current_bus_type;
+
+	/**
+	 * @tx_buf9: Buffer used for Option 1 9-bit conversion
+	 */
+	void *tx_buf9;
+
+	/**
+	 * @tx_buf9_len: Size of tx_buf9.
+	 */
+	size_t tx_buf9_len;
+};
+
+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_spi1_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;
+	struct spi_transfer tr = {
+		.bits_per_word = 9,
+	};
+	const u8 *src8 = msg->tx_buf;
+	struct spi_message m;
+	size_t max_chunk, chunk;
+	size_t len = msg->tx_len;
+	bool cmd_byte = true;
+	unsigned int i;
+	u16 *dst16;
+	int ret;
+
+	tr.speed_hz = mipi_dbi_spi_cmd_max_speed(spi, len);
+	dst16 = dbi->tx_buf9;
+
+	max_chunk = min(dbi->tx_buf9_len / 2, len);
+
+	spi_message_init_with_transfers(&m, &tr, 1);
+	tr.tx_buf = dst16;
+
+	while (len) {
+		chunk = min(len, max_chunk);
+
+		for (i = 0; i < chunk; i++) {
+			dst16[i] = *src8++;
+
+			/* Bit 9: 0 means command, 1 means data */
+			if (!cmd_byte)
+				dst16[i] |= BIT(9);
+
+			cmd_byte = false;
+		}
+
+		tr.len = chunk * 2;
+		len -= chunk;
+
+		ret = spi_sync(spi, &m);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static ssize_t dbi_spi3_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 -EINVAL;
+
+	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 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);
+
+	switch (dbi->current_bus_type) {
+	case MIPI_DCS_BUS_TYPE_DBI_SPI_C1:
+		return dbi_spi1_transfer(host, msg);
+	case MIPI_DCS_BUS_TYPE_DBI_SPI_C3:
+		return dbi_spi3_transfer(host, msg);
+	default:
+		dev_err(&dbi->spi->dev, "Unknown transfer type\n");
+		return -EINVAL;
+	}
+}
+
+static int dbi_spi_attach(struct mipi_dsi_host *host,
+			  struct mipi_dsi_device *dsi)
+{
+	struct dbi_spi *dbi = host_to_dbi_spi(host);
+
+	dbi->current_bus_type = dsi->bus_type;
+
+	if (dbi->current_bus_type == MIPI_DCS_BUS_TYPE_DBI_SPI_C1) {
+		dbi->tx_buf9_len = SZ_16K;
+
+		dbi->tx_buf9 = kmalloc(dbi->tx_buf9_len, GFP_KERNEL);
+		if (!dbi->tx_buf9)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int dbi_spi_detach(struct mipi_dsi_host *host,
+			  struct mipi_dsi_device *dsi)
+{
+	struct dbi_spi *dbi = host_to_dbi_spi(host);
+
+	kfree(dbi->tx_buf9);
+	dbi->tx_buf9_len = 0;
+
+	return 0; /* Nothing to do */
+}
+
+static void dbi_spi_host_unregister(void *d)
+{
+	mipi_dsi_host_unregister(d);
+}
+
+int drm_mipi_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;
+
+	dbi->host.dev = dev;
+	dbi->host.ops = &dbi->host_ops;
+	dbi->spi = spi;
+	spi_set_drvdata(spi, dbi);
+
+	dbi->dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dbi->dc)) {
+		dev_err(dev, "Failed to get gpio 'dc'\n");
+		return PTR_ERR(dbi->dc);
+	}
+
+	if (spi_is_bpw_supported(spi, 9))
+		dbi->host.bus_types |= MIPI_DCS_BUS_TYPE_DBI_SPI_C1;
+	if (dbi->dc)
+		dbi->host.bus_types |= MIPI_DCS_BUS_TYPE_DBI_SPI_C3;
+
+	if (!dbi->host.bus_types) {
+		dev_err(dev, "Neither Type1 nor Type3 are supported\n");
+		return -EINVAL;
+	}
+
+	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;
+}
+EXPORT_SYMBOL_GPL(drm_mipi_dbi_spi_probe);
+
+MODULE_DESCRIPTION("DBI SPI bus driver");
+MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_LICENSE("GPL");
diff --git a/include/drm/drm_mipi_dbi_spi.h b/include/drm/drm_mipi_dbi_spi.h
new file mode 100644
index 000000000000..ed3b040b1e4e
--- /dev/null
+++ b/include/drm/drm_mipi_dbi_spi.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * MIPI Display Bus Interface (DBI) LCD controller support
+ *
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#ifndef __DRM_MIPI_DBI_SPI_H
+#define __DRM_MIPI_DBI_SPI_H
+
+#include <drm/drm_mipi_dsi.h>
+#include <linux/spi/spi.h>
+
+int drm_mipi_dbi_spi_probe(struct spi_device *spi);
+
+#define module_mipi_dbi_spi_driver(__mipi_dbi_driver, __mipi_dbi_match_table) \
+	static struct spi_driver __mipi_dbi_driver##_spi __maybe_unused = { \
+		.driver = { \
+			.name = #__mipi_dbi_driver "_spi", \
+			.of_match_table = __mipi_dbi_match_table, \
+		}, \
+		.probe = drm_mipi_dbi_spi_probe, \
+	}; \
+	static int __mipi_dbi_driver##_module_init(void) \
+	{ \
+		if (IS_ENABLED(CONFIG_DRM_MIPI_DBI_SPI)) { \
+			int err = spi_register_driver(&__mipi_dbi_driver##_spi); \
+			if (err) \
+				return err; \
+		} \
+		return mipi_dsi_driver_register(&__mipi_dbi_driver); \
+	} \
+	static void __mipi_dbi_driver##_module_exit(void) \
+	{ \
+		mipi_dsi_driver_unregister(&__mipi_dbi_driver); \
+		if (IS_ENABLED(CONFIG_DRM_MIPI_DBI_SPI)) \
+			spi_unregister_driver(&__mipi_dbi_driver##_spi); \
+	} \
+	module_init(__mipi_dbi_driver##_module_init); \
+	module_exit(__mipi_dbi_driver##_module_exit)
+
+#endif /* __DRM_MIPI_DBI_SPI_H */
-- 
2.28.0

Re: [PATCH v2 3/6] drm: Add SPI DBI host driver

[Linus Walleij]

Hi Paul,

I looked a bit at this patch

On Sat, Aug 22, 2020 at 6:33 PM Paul Cercueil <paul@crapouillou.net> wrote:

> +config DRM_MIPI_DBI_SPI
> +       tristate "SPI host support for MIPI DBI"
> +       depends on DRM && OF && SPI

I think you want to depend on SPI_HOST actually.

> +       struct gpio_desc *dc;

This dc is very much undocumented, so I can only guess what
it is for, please add some kerneldoc explaining what it is.
I suppose it is in the DBI spec but I don't have it.

> +       gpiod_set_value_cansleep(dbi->dc, 0);

Since it is optional I usually prefer to do it like this:

if (dbi->dc)
   gpiod_set_value_cansleep(dbi->dc, 0);

> +  gpiod_set_value_cansleep(dbi->dc, 1);

Since you drive this low when you assert it and
high when you de-assert it, inverse this and mark the
GPIO line as GPIO_ACTIVE_LOW in the device tree
instead.

> +       dbi->dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
> +       if (IS_ERR(dbi->dc)) {
> +               dev_err(dev, "Failed to get gpio 'dc'\n");
> +               return PTR_ERR(dbi->dc);
> +       }

Currently you are requesting the line asserted according to the
above logic. Is that what you want?

If you change the DT to GPIO_ACTIVE_LOW then this seems
correct.

But I am overall a bit curious about this "dc" thing. What is it
really? It seems suspiciously similar to a SPI chip select,
and then that is something that should be handled by the
SPI core and set as cs-gpios in the device tree instead.
It is certainly used exactly like a chip select as far as I can
tell.

Yours,
Linus Walleij

[PATCH v2 4/6] drm/tiny: 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.

v2: No change

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 | 266 ++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h      |  19 +++
 4 files changed, 294 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 2b6414f0fa75..b62427b88dfe 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -28,6 +28,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 6ae4e9e5a35f..2bfee13347a5 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -2,6 +2,7 @@
 
 obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus.o
 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..b24d49836125
--- /dev/null
+++ b/drivers/gpu/drm/tiny/tiny-dsi.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * TinyDRM driver for standard DSI/DBI panels
+ *
+ * Copyright 2020 Paul Cercueil <paul@crapouillou.net>
+ */
+
+#include <linux/dma-mapping.h>
+#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,
+};
+
+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,
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(tiny_dsi_fops);
+
+static struct drm_driver tiny_dsi_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.name			= "tiny-dsi",
+	.desc			= "Tiny DSI",
+	.date			= "20200605",
+	.major			= 1,
+	.minor			= 0,
+
+	.fops			= &tiny_dsi_fops,
+	DRM_GEM_CMA_DRIVER_OPS,
+};
+
+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;
+
+	/*
+	 * Even though it's not the SPI device that does DMA (the master does),
+	 * the dma mask is necessary for the dma_alloc_wc() in
+	 * drm_gem_cma_create(). The dma_addr returned will be a physical
+	 * address which might be different from the bus address, but this is
+	 * not a problem since the address will not be used.
+	 * The virtual address is used in the transfer and the SPI core
+	 * re-maps it on the SPI master device using the DMA streaming API
+	 * (spi_map_buf()).
+	 */
+	if (!dev->coherent_dma_mask) {
+		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret) {
+			dev_warn(dev, "Failed to set dma mask %d\n", ret);
+			return ret;
+		}
+	}
+
+	priv = devm_drm_dev_alloc(dev, &tiny_dsi_driver, struct tiny_dsi, drm);
+	if (!priv)
+		return ret;
+
+	priv->dsi = dsi;
+	drm = &priv->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_init(drm);
+
+	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 802644c4c0c4..ee9538633943 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;
@@ -350,4 +351,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.28.0

[PATCH v2 5/6] drm/panel: Add panel driver for NewVision NV3052C based LCDs

[Paul Cercueil]

This driver supports the NewVision NV3052C based LCDs. Right now, it
only supports the LeadTek LTK035C5444T 2.4" 640x480 TFT LCD panel, which
can be found in the Anbernic RG-350M handheld console.

v2: - Remove custom backlight handling
    - Use drm_panel_{disable,unprepare} instead of
      nv3052c_{disable,unprepare}
    - Silence error when backlight probe defers
    - Remove 'dev' field in priv structure, use drm_panel->dev instead

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

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 8d97d07c5871..45b003752d65 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -198,6 +198,15 @@ config DRM_PANEL_NEC_NL8048HL11
 	  panel (found on the Zoom2/3/3630 SDP boards). To compile this driver
 	  as a module, choose M here.
 
+config DRM_PANEL_NEWVISION_NV3052C
+	tristate "NewVision NV3052C DSI/SPI RGB panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for the panels built
+	  around the NewVision NV3052C display controller.
+
 config DRM_PANEL_NOVATEK_NT35510
 	tristate "Novatek NT35510 RGB panel driver"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 15a4e7752951..c01743cdc08b 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
 obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
+obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
 obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
new file mode 100644
index 000000000000..52fd2622b8d9
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -0,0 +1,510 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * NevVision NV3052C IPS LCD panel driver
+ *
+ * Copyright (C) 2020, Paul Cercueil <paul@crapouillou.net>
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_mipi_dbi_spi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+struct nv3052c_panel_info {
+	const struct drm_display_mode *display_modes;
+	unsigned int num_modes;
+	unsigned int bus_type;
+	u16 width_mm, height_mm;
+	u32 bus_format, bus_flags;
+};
+
+struct nv3052c_reg {
+	u8 cmd;
+	u8 value;
+};
+
+struct nv3052c {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+
+	struct regulator *supply;
+	const struct nv3052c_panel_info *panel_info;
+
+	struct gpio_desc *reset_gpio;
+};
+
+static const struct nv3052c_reg nv3052c_regs[] = {
+	{ 0xff, 0x30 },
+	{ 0xff, 0x52 },
+	{ 0xff, 0x01 },
+	{ 0xe3, 0x00 },
+	{ 0x40, 0x00 },
+	{ 0x03, 0x40 },
+	{ 0x04, 0x00 },
+	{ 0x05, 0x03 },
+	{ 0x08, 0x00 },
+	{ 0x09, 0x07 },
+	{ 0x0a, 0x01 },
+	{ 0x0b, 0x32 },
+	{ 0x0c, 0x32 },
+	{ 0x0d, 0x0b },
+	{ 0x0e, 0x00 },
+	{ 0x23, 0xa0 },
+
+	{ 0x24, 0x0c },
+	{ 0x25, 0x06 },
+	{ 0x26, 0x14 },
+	{ 0x27, 0x14 },
+
+	{ 0x38, 0xcc },
+	{ 0x39, 0xd7 },
+	{ 0x3a, 0x4a },
+
+	{ 0x28, 0x40 },
+	{ 0x29, 0x01 },
+	{ 0x2a, 0xdf },
+	{ 0x49, 0x3c },
+	{ 0x91, 0x77 },
+	{ 0x92, 0x77 },
+	{ 0xa0, 0x55 },
+	{ 0xa1, 0x50 },
+	{ 0xa4, 0x9c },
+	{ 0xa7, 0x02 },
+	{ 0xa8, 0x01 },
+	{ 0xa9, 0x01 },
+	{ 0xaa, 0xfc },
+	{ 0xab, 0x28 },
+	{ 0xac, 0x06 },
+	{ 0xad, 0x06 },
+	{ 0xae, 0x06 },
+	{ 0xaf, 0x03 },
+	{ 0xb0, 0x08 },
+	{ 0xb1, 0x26 },
+	{ 0xb2, 0x28 },
+	{ 0xb3, 0x28 },
+	{ 0xb4, 0x33 },
+	{ 0xb5, 0x08 },
+	{ 0xb6, 0x26 },
+	{ 0xb7, 0x08 },
+	{ 0xb8, 0x26 },
+	{ 0xf0, 0x00 },
+	{ 0xf6, 0xc0 },
+
+	{ 0xff, 0x30 },
+	{ 0xff, 0x52 },
+	{ 0xff, 0x02 },
+	{ 0xb0, 0x0b },
+	{ 0xb1, 0x16 },
+	{ 0xb2, 0x17 },
+	{ 0xb3, 0x2c },
+	{ 0xb4, 0x32 },
+	{ 0xb5, 0x3b },
+	{ 0xb6, 0x29 },
+	{ 0xb7, 0x40 },
+	{ 0xb8, 0x0d },
+	{ 0xb9, 0x05 },
+	{ 0xba, 0x12 },
+	{ 0xbb, 0x10 },
+	{ 0xbc, 0x12 },
+	{ 0xbd, 0x15 },
+	{ 0xbe, 0x19 },
+	{ 0xbf, 0x0e },
+	{ 0xc0, 0x16 },
+	{ 0xc1, 0x0a },
+	{ 0xd0, 0x0c },
+	{ 0xd1, 0x17 },
+	{ 0xd2, 0x14 },
+	{ 0xd3, 0x2e },
+	{ 0xd4, 0x32 },
+	{ 0xd5, 0x3c },
+	{ 0xd6, 0x22 },
+	{ 0xd7, 0x3d },
+	{ 0xd8, 0x0d },
+	{ 0xd9, 0x07 },
+	{ 0xda, 0x13 },
+	{ 0xdb, 0x13 },
+	{ 0xdc, 0x11 },
+	{ 0xdd, 0x15 },
+	{ 0xde, 0x19 },
+	{ 0xdf, 0x10 },
+	{ 0xe0, 0x17 },
+	{ 0xe1, 0x0a },
+
+	{ 0xff, 0x30 },
+	{ 0xff, 0x52 },
+	{ 0xff, 0x03 },
+	{ 0x00, 0x2a },
+	{ 0x01, 0x2a },
+	{ 0x02, 0x2a },
+	{ 0x03, 0x2a },
+	{ 0x04, 0x61 },
+	{ 0x05, 0x80 },
+	{ 0x06, 0xc7 },
+	{ 0x07, 0x01 },
+	{ 0x08, 0x03 },
+	{ 0x09, 0x04 },
+	{ 0x70, 0x22 },
+	{ 0x71, 0x80 },
+	{ 0x30, 0x2a },
+	{ 0x31, 0x2a },
+	{ 0x32, 0x2a },
+	{ 0x33, 0x2a },
+	{ 0x34, 0x61 },
+	{ 0x35, 0xc5 },
+	{ 0x36, 0x80 },
+	{ 0x37, 0x23 },
+	{ 0x40, 0x03 },
+	{ 0x41, 0x04 },
+	{ 0x42, 0x05 },
+	{ 0x43, 0x06 },
+	{ 0x44, 0x11 },
+	{ 0x45, 0xe8 },
+	{ 0x46, 0xe9 },
+	{ 0x47, 0x11 },
+	{ 0x48, 0xea },
+	{ 0x49, 0xeb },
+	{ 0x50, 0x07 },
+	{ 0x51, 0x08 },
+	{ 0x52, 0x09 },
+	{ 0x53, 0x0a },
+	{ 0x54, 0x11 },
+	{ 0x55, 0xec },
+	{ 0x56, 0xed },
+	{ 0x57, 0x11 },
+	{ 0x58, 0xef },
+	{ 0x59, 0xf0 },
+	{ 0xb1, 0x01 },
+	{ 0xb4, 0x15 },
+	{ 0xb5, 0x16 },
+	{ 0xb6, 0x09 },
+	{ 0xb7, 0x0f },
+	{ 0xb8, 0x0d },
+	{ 0xb9, 0x0b },
+	{ 0xba, 0x00 },
+	{ 0xc7, 0x02 },
+	{ 0xca, 0x17 },
+	{ 0xcb, 0x18 },
+	{ 0xcc, 0x0a },
+	{ 0xcd, 0x10 },
+	{ 0xce, 0x0e },
+	{ 0xcf, 0x0c },
+	{ 0xd0, 0x00 },
+	{ 0x81, 0x00 },
+	{ 0x84, 0x15 },
+	{ 0x85, 0x16 },
+	{ 0x86, 0x10 },
+	{ 0x87, 0x0a },
+	{ 0x88, 0x0c },
+	{ 0x89, 0x0e },
+	{ 0x8a, 0x02 },
+	{ 0x97, 0x00 },
+	{ 0x9a, 0x17 },
+	{ 0x9b, 0x18 },
+	{ 0x9c, 0x0f },
+	{ 0x9d, 0x09 },
+	{ 0x9e, 0x0b },
+	{ 0x9f, 0x0d },
+	{ 0xa0, 0x01 },
+
+	{ 0xff, 0x30 },
+	{ 0xff, 0x52 },
+	{ 0xff, 0x02 },
+	{ 0x01, 0x01 },
+	{ 0x02, 0xda },
+	{ 0x03, 0xba },
+	{ 0x04, 0xa8 },
+	{ 0x05, 0x9a },
+	{ 0x06, 0x70 },
+	{ 0x07, 0xff },
+	{ 0x08, 0x91 },
+	{ 0x09, 0x90 },
+	{ 0x0a, 0xff },
+	{ 0x0b, 0x8f },
+	{ 0x0c, 0x60 },
+	{ 0x0d, 0x58 },
+	{ 0x0e, 0x48 },
+	{ 0x0f, 0x38 },
+	{ 0x10, 0x2b },
+
+	{ 0xff, 0x30 },
+	{ 0xff, 0x52 },
+	{ 0xff, 0x00 },
+	{ 0x36, 0x0a },
+};
+
+static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
+{
+	return container_of(panel, struct nv3052c, panel);
+}
+
+static int nv3052c_prepare(struct drm_panel *panel)
+{
+	struct nv3052c *priv = to_nv3052c(panel);
+	unsigned int i;
+	int err;
+
+	err = regulator_enable(priv->supply);
+	if (err) {
+		dev_err(panel->dev, "Failed to enable power supply: %d\n", err);
+		return err;
+	}
+
+	/* Reset the chip */
+	gpiod_set_value_cansleep(priv->reset_gpio, 1);
+	usleep_range(10, 1000);
+	gpiod_set_value_cansleep(priv->reset_gpio, 0);
+	usleep_range(5000, 20000);
+
+	for (i = 0; i < ARRAY_SIZE(nv3052c_regs); i++) {
+		err = mipi_dsi_dcs_write(priv->dsi, nv3052c_regs[i].cmd,
+					 &nv3052c_regs[i].value, 1);
+		if (err) {
+			dev_err(panel->dev, "Unable to set register: %d\n", err);
+			goto err_disable_regulator;
+		}
+	}
+
+	err = mipi_dsi_dcs_exit_sleep_mode(priv->dsi);
+	if (err) {
+		dev_err(panel->dev, "Unable to exit sleep mode: %d\n", err);
+		return err;
+	}
+
+	msleep(100);
+
+	return 0;
+
+err_disable_regulator:
+	regulator_disable(priv->supply);
+	return err;
+}
+
+static int nv3052c_unprepare(struct drm_panel *panel)
+{
+	struct nv3052c *priv = to_nv3052c(panel);
+	int err;
+
+	err = mipi_dsi_dcs_enter_sleep_mode(priv->dsi);
+	if (err) {
+		dev_err(panel->dev, "Unable to enter sleep mode: %d\n", err);
+		return err;
+	}
+
+	gpiod_set_value_cansleep(priv->reset_gpio, 1);
+
+	regulator_disable(priv->supply);
+
+	return 0;
+}
+
+static int nv3052c_enable(struct drm_panel *panel)
+{
+	struct nv3052c *priv = to_nv3052c(panel);
+	int err;
+
+	err = mipi_dsi_dcs_set_display_on(priv->dsi);
+	if (err) {
+		dev_err(panel->dev, "Unable to enable display: %d\n", err);
+		return err;
+	}
+
+	if (panel->backlight) {
+		/* Wait for the picture to be ready before enabling backlight */
+		usleep_range(10000, 20000);
+	}
+
+	return 0;
+}
+
+static int nv3052c_disable(struct drm_panel *panel)
+{
+	struct nv3052c *priv = to_nv3052c(panel);
+	int err;
+
+	err = mipi_dsi_dcs_set_display_off(priv->dsi);
+	if (err) {
+		dev_err(panel->dev, "Unable to disable display: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static int nv3052c_get_modes(struct drm_panel *panel,
+			     struct drm_connector *connector)
+{
+	struct nv3052c *priv = to_nv3052c(panel);
+	const struct nv3052c_panel_info *panel_info = priv->panel_info;
+	struct drm_display_mode *mode;
+	unsigned int i;
+
+	for (i = 0; i < panel_info->num_modes; i++) {
+		mode = drm_mode_duplicate(connector->dev,
+					  &panel_info->display_modes[i]);
+		if (!mode)
+			return -ENOMEM;
+
+		drm_mode_set_name(mode);
+
+		mode->type = DRM_MODE_TYPE_DRIVER;
+		if (panel_info->num_modes == 1)
+			mode->type |= DRM_MODE_TYPE_PREFERRED;
+
+		drm_mode_probed_add(connector, mode);
+	}
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = panel_info->width_mm;
+	connector->display_info.height_mm = panel_info->height_mm;
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &panel_info->bus_format, 1);
+	connector->display_info.bus_flags = panel_info->bus_flags;
+
+	return panel_info->num_modes;
+}
+
+static const struct drm_panel_funcs nv3052c_funcs = {
+	.prepare	= nv3052c_prepare,
+	.unprepare	= nv3052c_unprepare,
+	.enable		= nv3052c_enable,
+	.disable	= nv3052c_disable,
+	.get_modes	= nv3052c_get_modes,
+};
+
+static int nv3052c_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct nv3052c *priv;
+	int err;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dsi = dsi;
+	mipi_dsi_set_drvdata(dsi, priv);
+
+	priv->panel_info = of_device_get_match_data(dev);
+	if (!priv->panel_info)
+		return -EINVAL;
+
+	dsi->bus_type = priv->panel_info->bus_type;
+
+	priv->supply = devm_regulator_get(dev, "power");
+	if (IS_ERR(priv->supply)) {
+		dev_err(dev, "Failed to get power supply\n");
+		return PTR_ERR(priv->supply);
+	}
+
+	priv->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset_gpio)) {
+		dev_err(dev, "Failed to get reset GPIO\n");
+		return PTR_ERR(priv->reset_gpio);
+	}
+
+	drm_panel_init(&priv->panel, dev, &nv3052c_funcs,
+		       DRM_MODE_CONNECTOR_DPI);
+
+	err = drm_panel_of_backlight(&priv->panel);
+	if (err < 0) {
+		if (err != -EPROBE_DEFER)
+			dev_err(dev, "Failed to attach backlight\n");
+		return err;
+	}
+
+	drm_panel_add(&priv->panel);
+
+	err = mipi_dsi_attach(dsi);
+	if (err) {
+		dev_err(dev, "Failed to attach panel\n");
+		drm_panel_remove(&priv->panel);
+		return err;
+	}
+
+	/*
+	 * We can't call mipi_dsi_maybe_register_tiny_driver(), since the
+	 * NV3052C does not support MIPI_DCS_WRITE_MEMORY_START.
+	 */
+
+	return 0;
+}
+
+static int nv3052c_remove(struct mipi_dsi_device *dsi)
+{
+	struct nv3052c *priv = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&priv->panel);
+
+	drm_panel_disable(&priv->panel);
+	drm_panel_unprepare(&priv->panel);
+
+	return 0;
+}
+
+static const struct drm_display_mode ltk035c5444t_modes[] = {
+	{ /* 60 Hz */
+		.clock = 24000,
+		.hdisplay = 640,
+		.hsync_start = 640 + 96,
+		.hsync_end = 640 + 96 + 16,
+		.htotal = 640 + 96 + 16 + 48,
+		.vdisplay = 480,
+		.vsync_start = 480 + 5,
+		.vsync_end = 480 + 5 + 2,
+		.vtotal = 480 + 5 + 2 + 13,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+	},
+	{ /* 50 Hz */
+		.clock = 18000,
+		.hdisplay = 640,
+		.hsync_start = 640 + 39,
+		.hsync_end = 640 + 39 + 2,
+		.htotal = 640 + 39 + 2 + 39,
+		.vdisplay = 480,
+		.vsync_start = 480 + 5,
+		.vsync_end = 480 + 5 + 2,
+		.vtotal = 480 + 5 + 2 + 13,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+	},
+};
+
+static const struct nv3052c_panel_info ltk035c5444t_spi_panel_info = {
+	.display_modes = ltk035c5444t_modes,
+	.num_modes = ARRAY_SIZE(ltk035c5444t_modes),
+	.bus_type = MIPI_DCS_BUS_TYPE_DBI_SPI_C1,
+	.width_mm = 77,
+	.height_mm = 64,
+	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE,
+};
+
+static const struct of_device_id nv3052c_of_match[] = {
+	{ .compatible = "leadtek,ltk035c5444t-spi", .data = &ltk035c5444t_spi_panel_info },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, nv3052c_of_match);
+
+static struct mipi_dsi_driver nv3052c_driver = {
+	.driver = {
+		.name = "nv3052c",
+		.of_match_table = nv3052c_of_match,
+	},
+	.probe = nv3052c_probe,
+	.remove = nv3052c_remove,
+};
+module_mipi_dbi_spi_driver(nv3052c_driver, nv3052c_of_match);
+
+MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_LICENSE("GPL v2");
-- 
2.28.0

Re: [PATCH v2 5/6] drm/panel: Add panel driver for NewVision NV3052C based LCDs

[Linus Walleij]

On Sat, Aug 22, 2020 at 6:33 PM Paul Cercueil <paul@crapouillou.net> wrote:

> +static const struct nv3052c_reg nv3052c_regs[] = {
> +       { 0xff, 0x30 },
> +       { 0xff, 0x52 },
> +       { 0xff, 0x01 },
> +       { 0xe3, 0x00 },
> +       { 0x40, 0x00 },
(...)

Well that's pretty opaque :D

I suppose no datasheet (why do vendors keep doing this to us...)

In other kernel code I have referred to this as a "jam table", e.g.
drivers/net/dsa/rtl8366rb.c.

I didn't make this up, the name comes from Bunnie Huang's
book on hacking the Xbox, and he says it is common hardware
engineer lingo.
https://www.iacr.org/workshops/ches/ches2002/presentations/Huang.pdf

What about naming it nv3052c_jam_table[] or nv3052c_jam[]?

Yours,
Linus Walleij

[PATCH v2 6/6] drm/panel: 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.

v2: - Remove custom handling of backlight
    - Call drm_panel_disable() / drm_panel_unprepare() on module exit
    - drm_panel_add() is a void function now

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 | 318 +++++++++++++++++++
 3 files changed, 328 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 45b003752d65..38c581e91986 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -105,6 +105,15 @@ config DRM_PANEL_ILITEK_IL9322
 	  Say Y here if you want to enable support for Ilitek IL9322
 	  QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
 
+config DRM_PANEL_ILITEK_ILI9341
+	tristate "Ilitek ILI9341 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 c01743cdc08b..d732c2b8a747 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -9,6 +9,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_ILI9341) += panel-ilitek-ili9341.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
 obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
new file mode 100644
index 000000000000..8f32edaac627
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -0,0 +1,318 @@
+// 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/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;
+	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_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%u\n",
+			priv->pdata->mode.hdisplay, priv->pdata->mode.vdisplay);
+		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_SAMPLE_POSEDGE;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ili9341_funcs = {
+	.prepare	= ili9341_prepare,
+	.unprepare	= ili9341_unprepare,
+	.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);
+	}
+
+	ret = drm_panel_of_backlight(&priv->panel);
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get backlight handle\n");
+		return ret;
+	}
+
+	drm_panel_add(&priv->panel);
+
+	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);
+
+	drm_panel_disable(&priv->panel);
+	drm_panel_unprepare(&priv->panel);
+
+	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_DCS_BUS_TYPE_DBI_SPI_C3,
+	.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.28.0

答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[何小龙 (Leon He)]

> +struct ili9341 {
> +       struct drm_panel panel;
> +       struct mipi_dsi_device *dsi;
> +       const struct ili9341_pdata *pdata;
> +
> +       struct gpio_desc        *reset_gpiod;
> +       u32 rotation;
> +};
> +

Hi Paul, you put the mipi_dsi_device inside the struct. I think it maybe not
a good idea. That means the panel has a MIPI-DSI interface but it doesn't
have actually.

> +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);
> +       }
> +
> +       ret = drm_panel_of_backlight(&priv->panel);
> +       if (ret < 0) {
> +               if (ret != -EPROBE_DEFER)
> +                       dev_err(dev, "Failed to get backlight handle\n");
> +               return ret;
> +       }
> +
> +       drm_panel_add(&priv->panel);
> +
> +       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);
> +
> +       drm_panel_disable(&priv->panel);
> +       drm_panel_unprepare(&priv->panel);
> +
> +       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_DCS_BUS_TYPE_DBI_SPI_C3,
> +       .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);

Again, you treat this driver as a mipi dsi driver but for a MIPI-DBI (I8080/SPI)
panel device. That will make developers confused.

Is it possible to just add a mipi_dbi_driver for I8080/SPI interface panel?
Thanks!


Best regards

________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

Re: 答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[Paul Cercueil]

Hi Leon,

Le dim. 30 août 2020 à 16:36, 何小龙 (Leon He) 
<Leon.He@unisoc.com> a écrit :
>>  +struct ili9341 {
>>  +       struct drm_panel panel;
>>  +       struct mipi_dsi_device *dsi;
>>  +       const struct ili9341_pdata *pdata;
>>  +
>>  +       struct gpio_desc        *reset_gpiod;
>>  +       u32 rotation;
>>  +};
>>  +
> 
> Hi Paul, you put the mipi_dsi_device inside the struct. I think it 
> maybe not
> a good idea. That means the panel has a MIPI-DSI interface but it 
> doesn't
> have actually.
> 
>>  +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);
>>  +       }
>>  +
>>  +       ret = drm_panel_of_backlight(&priv->panel);
>>  +       if (ret < 0) {
>>  +               if (ret != -EPROBE_DEFER)
>>  +                       dev_err(dev, "Failed to get backlight 
>> handle\n");
>>  +               return ret;
>>  +       }
>>  +
>>  +       drm_panel_add(&priv->panel);
>>  +
>>  +       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);
>>  +
>>  +       drm_panel_disable(&priv->panel);
>>  +       drm_panel_unprepare(&priv->panel);
>>  +
>>  +       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_DCS_BUS_TYPE_DBI_SPI_C3,
>>  +       .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);
> 
> Again, you treat this driver as a mipi dsi driver but for a MIPI-DBI 
> (I8080/SPI)
> panel device. That will make developers confused.
> 
> Is it possible to just add a mipi_dbi_driver for I8080/SPI interface 
> panel?
> Thanks!

Please read the cover letter, it explains why it's done this way. The 
whole point of this patchset is to merge DSI and DBI frameworks in a 
way that can be maintained.

Cheers,
-Paul

Re: 答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[Laurent Pinchart]

Hi Paul,

On Sun, Aug 30, 2020 at 06:48:12PM +0200, Paul Cercueil wrote:
> Le dim. 30 août 2020 à 16:36, 何小龙 (Leon He) a écrit :
> >>  +struct ili9341 {
> >>  +       struct drm_panel panel;
> >>  +       struct mipi_dsi_device *dsi;
> >>  +       const struct ili9341_pdata *pdata;
> >>  +
> >>  +       struct gpio_desc        *reset_gpiod;
> >>  +       u32 rotation;
> >>  +};
> >>  +
> > 
> > Hi Paul, you put the mipi_dsi_device inside the struct. I think it 
> > maybe not
> > a good idea. That means the panel has a MIPI-DSI interface but it 
> > doesn't
> > have actually.
> > 
> >>  +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);
> >>  +       }
> >>  +
> >>  +       ret = drm_panel_of_backlight(&priv->panel);
> >>  +       if (ret < 0) {
> >>  +               if (ret != -EPROBE_DEFER)
> >>  +                       dev_err(dev, "Failed to get backlight 
> >> handle\n");
> >>  +               return ret;
> >>  +       }
> >>  +
> >>  +       drm_panel_add(&priv->panel);
> >>  +
> >>  +       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);
> >>  +
> >>  +       drm_panel_disable(&priv->panel);
> >>  +       drm_panel_unprepare(&priv->panel);
> >>  +
> >>  +       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_DCS_BUS_TYPE_DBI_SPI_C3,
> >>  +       .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);
> > 
> > Again, you treat this driver as a mipi dsi driver but for a MIPI-DBI 
> > (I8080/SPI)
> > panel device. That will make developers confused.
> > 
> > Is it possible to just add a mipi_dbi_driver for I8080/SPI interface 
> > panel?
> > Thanks!
> 
> Please read the cover letter, it explains why it's done this way. The 
> whole point of this patchset is to merge DSI and DBI frameworks in a 
> way that can be maintained.

I think this proves the point that the proposed naming is confusing. At
least a rename would be required.

-- 
Regards,

Laurent Pinchart

Re: 答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[Sam Ravnborg]

Hi Laurent.

> > 
> > Please read the cover letter, it explains why it's done this way. The 
> > whole point of this patchset is to merge DSI and DBI frameworks in a 
> > way that can be maintained.
> 
> I think this proves the point that the proposed naming is confusing. At
> least a rename would be required.

Do you have any inputs on the amount of rename we are looking into.
Is this a simple s/struct mipi_dsi_device/struct mipi_dxi_device/
or something more?

We should script the rename as it will tocuh a lot of files,
and without a script we would chase this. But once it is scripted
it would be trivial to perform.

I did not look at this enough, but I had an idea that we
would have do to a s/dsi/dxi/ in a lot of places.

(dxi is my best proposal at the moment for something covering both dsi
and dbi).

PS. I am travelling for a few days, so do not expect quick responses.

	Sam

Re: 答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[Paul Cercueil]

Le dim. 30 août 2020 à 22:28, Sam Ravnborg <sam@ravnborg.org> a 
écrit :
> Hi Laurent.
> 
>>  >
>>  > Please read the cover letter, it explains why it's done this way. 
>> The
>>  > whole point of this patchset is to merge DSI and DBI frameworks 
>> in a
>>  > way that can be maintained.
>> 
>>  I think this proves the point that the proposed naming is 
>> confusing. At
>>  least a rename would be required.
> 
> Do you have any inputs on the amount of rename we are looking into.
> Is this a simple s/struct mipi_dsi_device/struct mipi_dxi_device/
> or something more?
> 
> We should script the rename as it will tocuh a lot of files,
> and without a script we would chase this. But once it is scripted
> it would be trivial to perform.
> 
> I did not look at this enough, but I had an idea that we
> would have do to a s/dsi/dxi/ in a lot of places.
> 
> (dxi is my best proposal at the moment for something covering both dsi
> and dbi).

dcs?

Since DBI and DSI panels generally all use DCS commands.

-Paul

Re: 答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[Neil Armstrong]

On 07/09/2020 14:57, Paul Cercueil wrote:
> 
> 
> Le dim. 30 août 2020 à 22:28, Sam Ravnborg <sam@ravnborg.org> a écrit :
>> Hi Laurent.
>>
>>>  >
>>>  > Please read the cover letter, it explains why it's done this way. The
>>>  > whole point of this patchset is to merge DSI and DBI frameworks in a
>>>  > way that can be maintained.
>>>
>>>  I think this proves the point that the proposed naming is confusing. At
>>>  least a rename would be required.
>>
>> Do you have any inputs on the amount of rename we are looking into.
>> Is this a simple s/struct mipi_dsi_device/struct mipi_dxi_device/
>> or something more?
>>
>> We should script the rename as it will tocuh a lot of files,
>> and without a script we would chase this. But once it is scripted
>> it would be trivial to perform.
>>
>> I did not look at this enough, but I had an idea that we
>> would have do to a s/dsi/dxi/ in a lot of places.
>>
>> (dxi is my best proposal at the moment for something covering both dsi
>> and dbi).
> 
> dcs?
> 
> Since DBI and DSI panels generally all use DCS commands.

mipi_disp / mipi_display ? since it's all about mipi display interfaces
with different transport protocols.

Neil

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

Re: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

[Linus Walleij]

Hi Paul,

just a drive-by comment:

On Sat, Aug 22, 2020 at 6:33 PM Paul Cercueil <paul@crapouillou.net> wrote:

> +       gpiod_set_value_cansleep(priv->reset_gpiod, 0);
> +       usleep_range(20, 1000);
> +       gpiod_set_value_cansleep(priv->reset_gpiod, 1);

This implies that the reset line is active low.

I would specify in the DT GPIO handle that it is active low
and invert the above.

So:

reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;

gpiod_set_value_cansleep(priv->reset_gpiod, 1);
usleep_range(20, 1000);
gpiod_set_value_cansleep(priv->reset_gpiod, 0);

> +       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);
> +       }

This would then fetch the GPIO as asserted (device in reset)
unless changed, but that may be the right thing to do actually.

> +static const struct ili9341_pdata yx240qv29_pdata = {
> +       .mode = { DRM_SIMPLE_MODE(240, 320, 37, 49) },
> +       .width_mm = 0, // TODO
> +       .height_mm = 0, // TODO

When nothing else works and data sheets are incomplete I
just take out a ruler and measure on the actual device.

Yours,
Linus Walleij