[PATCH 0/3] drm/panel: Add MIPI DBI compatible SPI driver

[PATCH 0/3] drm/panel: Add MIPI DBI compatible SPI driver

[Noralf Trønnes]

Hi,

This patchset adds a driver that will work with most MIPI DBI compatible
SPI panels out there.

It's a follow up on 'drm/tiny/st7735r: Match up with staging/fbtft
driver'[1] which aimed at making the st7735r driver work with all panels
adding DT properties.

Maxime gave[2] a good overview of the situation with these displays and
proposed to make a driver that works with all MIPI DBI compatible
controllers and use a firmware file to provide the controller setup for
a particular panel.

Noralf.

[1] https://lore.kernel.org/dri-devel/20211124150757.17929-1-noralf@tronnes.org/
[2] https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/


Noralf Trønnes (3):
  dt-bindings: display: add bindings for MIPI DBI compatible SPI panels
  drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev
  drm/panel: Add MIPI DBI compatible SPI driver

 .../display/panel/panel-mipi-dbi-spi.yaml     |  69 ++++
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-mipi-dbi.c        | 385 ++++++++++++++++++
 include/drm/drm_mipi_dbi.h                    |   2 +
 6 files changed, 475 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-mipi-dbi.c

-- 
2.33.0

[PATCH 0/3] drm/panel: Add MIPI DBI compatible SPI driver

[Noralf Trønnes]

Hi,

This patchset adds a driver that will work with most MIPI DBI compatible
SPI panels out there.

It's a follow up on 'drm/tiny/st7735r: Match up with staging/fbtft
driver'[1] which aimed at making the st7735r driver work with all panels
adding DT properties.

Maxime gave[2] a good overview of the situation with these displays and
proposed to make a driver that works with all MIPI DBI compatible
controllers and use a firmware file to provide the controller setup for
a particular panel.

Noralf.

[1] https://lore.kernel.org/dri-devel/20211124150757.17929-1-noralf@tronnes.org/
[2] https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/


Noralf Trønnes (3):
  dt-bindings: display: add bindings for MIPI DBI compatible SPI panels
  drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev
  drm/panel: Add MIPI DBI compatible SPI driver

 .../display/panel/panel-mipi-dbi-spi.yaml     |  69 ++++
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-mipi-dbi.c        | 385 ++++++++++++++++++
 include/drm/drm_mipi_dbi.h                    |   2 +
 6 files changed, 475 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-mipi-dbi.c

-- 
2.33.0

[PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Add binding for MIPI DBI compatible SPI panels.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
new file mode 100644
index 000000000000..d6c8accb045c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPI DBI SPI Panels Device Tree Bindings
+
+maintainers:
+  - Noralf Trønnes <noralf@tronnes.org>
+
+description:
+  This binding is for display panels using a MIPI DBI controller
+  in SPI mode.
+
+allOf:
+  - $ref: panel/panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: panel-mipi-dbi-spi
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: The name of the display panel.
+
+  write-only:
+    type: boolean
+    description:
+      Controller is not readable (ie. MISO is not wired up).
+
+  dc-gpios:
+    maxItems: 1
+    description: |
+      Controller data/command selection (D/CX) in 4-line SPI mode.
+      If not set, the controller is in 3-line SPI mode.
+
+  backlight: true
+  reg: true
+  reset-gpios: true
+
+required:
+  - compatible
+  - model
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            display@0{
+                    compatible = "panel-mipi-dbi-spi";
+                    model = "sainsmart18";
+                    reg = <0>;
+                    spi-max-frequency = <40000000>;
+                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
+                    write-only;
+            };
+    };
+
+...
-- 
2.33.0

[PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Add binding for MIPI DBI compatible SPI panels.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
new file mode 100644
index 000000000000..d6c8accb045c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPI DBI SPI Panels Device Tree Bindings
+
+maintainers:
+  - Noralf Trønnes <noralf@tronnes.org>
+
+description:
+  This binding is for display panels using a MIPI DBI controller
+  in SPI mode.
+
+allOf:
+  - $ref: panel/panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: panel-mipi-dbi-spi
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: The name of the display panel.
+
+  write-only:
+    type: boolean
+    description:
+      Controller is not readable (ie. MISO is not wired up).
+
+  dc-gpios:
+    maxItems: 1
+    description: |
+      Controller data/command selection (D/CX) in 4-line SPI mode.
+      If not set, the controller is in 3-line SPI mode.
+
+  backlight: true
+  reg: true
+  reset-gpios: true
+
+required:
+  - compatible
+  - model
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            display@0{
+                    compatible = "panel-mipi-dbi-spi";
+                    model = "sainsmart18";
+                    reg = <0>;
+                    spi-max-frequency = <40000000>;
+                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
+                    write-only;
+            };
+    };
+
+...
-- 
2.33.0

[PATCH 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev

[Noralf Trønnes]

devm_drm_dev_alloc() can't allocate structures that embed a structure
which then again embeds drm_device. Workaround this by adding a
driver_private pointer to struct mipi_dbi_dev which the driver can use for
its additional state.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 include/drm/drm_mipi_dbi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 05e194958265..e24865058d87 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -130,6 +130,8 @@ struct mipi_dbi_dev {
 	 * @dbi: MIPI DBI interface
 	 */
 	struct mipi_dbi dbi;
+
+	void *driver_private;
 };
 
 static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm)
-- 
2.33.0

[PATCH 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev

[Noralf Trønnes]

devm_drm_dev_alloc() can't allocate structures that embed a structure
which then again embeds drm_device. Workaround this by adding a
driver_private pointer to struct mipi_dbi_dev which the driver can use for
its additional state.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 include/drm/drm_mipi_dbi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 05e194958265..e24865058d87 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -130,6 +130,8 @@ struct mipi_dbi_dev {
 	 * @dbi: MIPI DBI interface
 	 */
 	struct mipi_dbi dbi;
+
+	void *driver_private;
 };
 
 static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm)
-- 
2.33.0

[PATCH 3/3] drm/panel: Add MIPI DBI compatible SPI driver

[Noralf Trønnes]

Add a driver that will work with most MIPI DBI compatible SPI panels.
This avoids adding a driver for every new MIPI DBI compatible controller
that is to be used by Linux. The 'model' Device Tree property contains the
name of the display and will be used to load a firmware file that contains
the controller configuration.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 MAINTAINERS                            |   7 +
 drivers/gpu/drm/panel/Kconfig          |  11 +
 drivers/gpu/drm/panel/Makefile         |   1 +
 drivers/gpu/drm/panel/panel-mipi-dbi.c | 385 +++++++++++++++++++++++++
 4 files changed, 404 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-mipi-dbi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ba6fa65df8bb..8a46c36c50dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6035,6 +6035,13 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt
 F:	drivers/gpu/drm/tiny/mi0283qt.c
 
+DRM DRIVER FOR MIPI DBI compatible panels
+M:	Noralf Trønnes <noralf@tronnes.org>
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
+F:	drivers/gpu/drm/panel/panel-mipi-dbi.c
+
 DRM DRIVER FOR MSM ADRENO GPU
 M:	Rob Clark <robdclark@gmail.com>
 M:	Sean Paul <sean@poorly.run>
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 434c2861bb40..1851cda5f877 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -274,6 +274,17 @@ config DRM_PANEL_LG_LG4573
 	  Say Y here if you want to enable support for LG4573 RGB panel.
 	  To compile this driver as a module, choose M here.
 
+config DRM_PANEL_MIPI_DBI
+	tristate "MIPI DBI compatible panel"
+	depends on SPI
+	depends on BACKLIGHT_CLASS_DEVICE
+	depends on DRM_KMS_HELPER
+	select DRM_KMS_CMA_HELPER
+	select DRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for MIPI DBI compatible panels.
+	  To compile this driver as a module, choose M here.
+
 config DRM_PANEL_NEC_NL8048HL11
 	tristate "NEC NL8048HL11 RGB panel"
 	depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d99fbbce49d1..a90c30459964 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 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_MIPI_DBI) += panel-mipi-dbi.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35950) += panel-novatek-nt35950.o
diff --git a/drivers/gpu/drm/panel/panel-mipi-dbi.c b/drivers/gpu/drm/panel/panel-mipi-dbi.c
new file mode 100644
index 000000000000..767658426061
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-mipi-dbi.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DRM driver for MIPI DBI compatible display panels
+ *
+ * Copyright 2022 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_modeset_helper.h>
+#include <video/mipi_display.h>
+
+static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 'B', 'I',
+					     0, 0, 0, 0, 0, 0, 0 };
+
+/*
+ * The display panel configuration is stored in a firmware file. The Device Tree 'model' property
+ * value with a '.bin' suffix is passed to request_firmware() to fetch this file.
+ */
+struct panel_mipi_dbi_config {
+	/* Magic string: panel_mipi_dbi_magic */
+	u8 magic[15];
+
+	/* Config file format version */
+	u8 file_format_version;
+
+	/* Width in pixels */
+	__be16 width;
+	/* Height in pixels */
+	__be16 height;
+
+	/* Width in millimeters (optional) */
+	__be16 width_mm;
+	/* Height in millimeters (optional) */
+	__be16 height_mm;
+
+	/* X-axis panel offset */
+	__be16 x_offset;
+	/* Y-axis panel offset */
+	__be16 y_offset;
+
+	/* 4 pad bytes, must be zero */
+	u8 pad[4];
+
+	/*
+	 * Optional MIPI commands to execute when the display pipeline is enabled.
+	 * This can be used to configure the display controller.
+	 *
+	 * The commands are stored in a byte array with the format:
+	 *     command, num_parameters, [ parameter, ...], command, ...
+	 *
+	 * Some commands require a pause before the next command can be received.
+	 * Inserting a delay in the command sequence is done by using the NOP command with one
+	 * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display
+	 * Command Set where it has no parameters).
+	 *
+	 * Example:
+	 *     command 0x11
+	 *     sleep 120ms
+	 *     command 0xb1 parameters 0x01, 0x2c, 0x2d
+	 *     command 0x29
+	 *
+	 * Byte sequence:
+	 *     0x11 0x00
+	 *     0x00 0x01 0x78
+	 *     0xb1 0x03 0x01 0x2c 0x2d
+	 *     0x29 0x00
+	 */
+	u8 commands[];
+};
+
+struct panel_mipi_dbi_commands {
+	const u8 *buf;
+	size_t len;
+};
+
+static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe,
+				  struct drm_crtc_state *crtc_state,
+				  struct drm_plane_state *plane_state)
+{
+	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+	struct panel_mipi_dbi_commands *commands = dbidev->driver_private;
+	struct mipi_dbi *dbi = &dbidev->dbi;
+	unsigned int i = 0;
+	int ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
+
+	drm_dbg(pipe->crtc.dev, "\n");
+
+	ret = mipi_dbi_poweron_conditional_reset(dbidev);
+	if (ret < 0)
+		goto out_exit;
+	if (ret == 1)
+		goto out_enable;
+
+	if (!commands)
+		goto out_enable;
+
+	while (i < commands->len) {
+		u8 command = commands->buf[i++];
+		u8 num_parameters = commands->buf[i++];
+		const u8 *parameters = &commands->buf[i];
+
+		if (command == 0x00 && num_parameters == 1)
+			msleep(parameters[0]);
+		else if (num_parameters)
+			mipi_dbi_command_stackbuf(dbi, command, parameters, num_parameters);
+		else
+			mipi_dbi_command(dbi, command);
+
+		i += num_parameters;
+	}
+
+out_enable:
+	mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+out_exit:
+	drm_dev_exit(idx);
+}
+
+static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = {
+	.enable = panel_mipi_dbi_enable,
+	.disable = mipi_dbi_pipe_disable,
+	.update = mipi_dbi_pipe_update,
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(panel_mipi_dbi_fops);
+
+static const struct drm_driver panel_mipi_dbi_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.fops			= &panel_mipi_dbi_fops,
+	DRM_GEM_CMA_DRIVER_OPS_VMAP,
+	.debugfs_init		= mipi_dbi_debugfs_init,
+	.name			= "panel-mipi-dbi",
+	.desc			= "MIPI DBI compatible display panel",
+	.date			= "20220103",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static int panel_mipi_dbi_parse_config(struct mipi_dbi_dev *dbidev,
+				       struct drm_display_mode *mode,
+				       const struct firmware *fw)
+{
+	const struct panel_mipi_dbi_config *config = (struct panel_mipi_dbi_config *)fw->data;
+	unsigned int width, height, x_offset, y_offset;
+	struct panel_mipi_dbi_commands *commands;
+	struct drm_device *drm = &dbidev->drm;
+	struct device *dev = dbidev->drm.dev;
+	size_t size = fw->size, commands_len;
+	unsigned int i = 0;
+
+	if (size < sizeof(*config)) {
+		dev_err(dev, "config: file size=%zu is too small\n", size);
+		return -EINVAL;
+	}
+
+	if (memcmp(config->magic, panel_mipi_dbi_magic, sizeof(config->magic))) {
+		dev_err(dev, "config: Bad magic: %15ph\n", config->magic);
+		return -EINVAL;
+	}
+
+	if (config->file_format_version != 1) {
+		dev_err(dev, "config: version=%u is not supported\n", config->file_format_version);
+		return -EINVAL;
+	}
+
+	width = be16_to_cpu(config->width);
+	height = be16_to_cpu(config->height);
+	x_offset = be16_to_cpu(config->x_offset);
+	y_offset = be16_to_cpu(config->y_offset);
+
+	drm_dbg(drm, "size=%zu version=%u\n", size, config->file_format_version);
+	drm_dbg(drm, "width=%u height=%u\n", width, height);
+	drm_dbg(drm, "x_offset=%u y_offset=%u\n", x_offset, y_offset);
+
+	if (width && height) {
+		struct drm_display_mode simple_mode = {
+			DRM_SIMPLE_MODE(width, height, be16_to_cpu(config->width_mm),
+					be16_to_cpu(config->height_mm))
+		};
+
+		*mode = simple_mode;
+	} else {
+		dev_err(dev, "config: width or height can't be zero\n");
+		return -EINVAL;
+	}
+
+	dbidev->left_offset = x_offset;
+	dbidev->top_offset = y_offset;
+
+	commands_len = size - sizeof(*config);
+	if (!commands_len)
+		return 0;
+
+	while ((i + 1) < commands_len) {
+		u8 command = config->commands[i++];
+		u8 num_parameters = config->commands[i++];
+		const u8 *parameters = &config->commands[i];
+
+		i += num_parameters;
+		if (i > commands_len) {
+			dev_err(dev, "config: command=0x%02x num_parameters=%u overflows\n",
+				command, num_parameters);
+			return -EINVAL;
+		}
+
+		if (command == 0x00 && num_parameters == 1)
+			drm_dbg(drm, "sleep %ums\n", parameters[0]);
+		else
+			drm_dbg(drm, "command %02x %*ph\n", command, num_parameters, parameters);
+	}
+
+	if (i != commands_len) {
+		dev_err(dev, "config: malformed command array\n");
+		return -EINVAL;
+	}
+
+	commands = devm_kzalloc(dev, sizeof(*commands), GFP_KERNEL);
+	if (!commands)
+		return -ENOMEM;
+
+	commands->len = commands_len;
+	commands->buf = devm_kmemdup(dev, config->commands, commands->len, GFP_KERNEL);
+	if (!commands->buf)
+		return -ENOMEM;
+
+	dbidev->driver_private = commands;
+
+	return 0;
+}
+
+static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct drm_display_mode mode;
+	struct mipi_dbi_dev *dbidev;
+	const struct firmware *fw;
+	struct drm_device *drm;
+	struct mipi_dbi *dbi;
+	struct gpio_desc *dc;
+	const char *model;
+	char fw_name[40];
+	int ret;
+
+	ret = device_property_read_string(dev, "model", &model);
+	if (ret) {
+		dev_err(dev, "Failed to get 'model' property\n");
+		return ret;
+	}
+
+	dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm);
+	if (IS_ERR(dbidev))
+		return PTR_ERR(dbidev);
+
+	dbi = &dbidev->dbi;
+	drm = &dbidev->drm;
+
+	snprintf(fw_name, sizeof(fw_name), "%s.bin", model);
+
+	ret = request_firmware(&fw, fw_name, dev);
+	if (ret)
+		return ret;
+
+	ret = panel_mipi_dbi_parse_config(dbidev, &mode, fw);
+	release_firmware(fw);
+	if (ret)
+		return ret;
+
+	dbidev->regulator = devm_regulator_get(dev, "power");
+	if (IS_ERR(dbidev->regulator))
+		return dev_err_probe(dev, PTR_ERR(dbidev->regulator),
+				     "Failed to get regulator 'power'\n");
+
+	dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(dbi->reset))
+		return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n");
+
+	dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc))
+		return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n");
+
+	dbidev->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(dbidev->backlight))
+		return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n");
+
+	ret = mipi_dbi_spi_init(spi, dbi, dc);
+	if (ret)
+		return ret;
+
+	if (device_property_present(dev, "write-only"))
+		dbi->read_commands = NULL;
+
+	ret = mipi_dbi_dev_init(dbidev, &panel_mipi_dbi_pipe_funcs, &mode, 0);
+	if (ret)
+		return ret;
+
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 0);
+
+	return 0;
+}
+
+static int panel_mipi_dbi_spi_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
+}
+
+static void panel_mipi_dbi_spi_shutdown(struct spi_device *spi)
+{
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
+}
+
+static int __maybe_unused panel_mipi_dbi_pm_suspend(struct device *dev)
+{
+	return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
+}
+
+static int __maybe_unused panel_mipi_dbi_pm_resume(struct device *dev)
+{
+	drm_mode_config_helper_resume(dev_get_drvdata(dev));
+
+	return 0;
+}
+
+static const struct dev_pm_ops panel_mipi_dbi_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(panel_mipi_dbi_pm_suspend, panel_mipi_dbi_pm_resume)
+};
+
+static const struct of_device_id panel_mipi_dbi_spi_of_match[] = {
+	{ .compatible = "panel-mipi-dbi-spi" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, panel_mipi_dbi_spi_of_match);
+
+static const struct spi_device_id panel_mipi_dbi_spi_id[] = {
+	{ "panel-mipi-dbi-spi", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, panel_mipi_dbi_spi_id);
+
+static struct spi_driver panel_mipi_dbi_spi_driver = {
+	.driver = {
+		.name = "panel-mipi-dbi-spi",
+		.owner = THIS_MODULE,
+		.of_match_table = panel_mipi_dbi_spi_of_match,
+		.pm = &panel_mipi_dbi_pm_ops,
+	},
+	.id_table = panel_mipi_dbi_spi_id,
+	.probe = panel_mipi_dbi_spi_probe,
+	.remove = panel_mipi_dbi_spi_remove,
+	.shutdown = panel_mipi_dbi_spi_shutdown,
+};
+module_spi_driver(panel_mipi_dbi_spi_driver);
+
+MODULE_DESCRIPTION("MIPI DBI compatible display panel driver");
+MODULE_AUTHOR("Noralf Trønnes");
+MODULE_LICENSE("GPL");
-- 
2.33.0

[PATCH 3/3] drm/panel: Add MIPI DBI compatible SPI driver

[Noralf Trønnes]

Add a driver that will work with most MIPI DBI compatible SPI panels.
This avoids adding a driver for every new MIPI DBI compatible controller
that is to be used by Linux. The 'model' Device Tree property contains the
name of the display and will be used to load a firmware file that contains
the controller configuration.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 MAINTAINERS                            |   7 +
 drivers/gpu/drm/panel/Kconfig          |  11 +
 drivers/gpu/drm/panel/Makefile         |   1 +
 drivers/gpu/drm/panel/panel-mipi-dbi.c | 385 +++++++++++++++++++++++++
 4 files changed, 404 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-mipi-dbi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ba6fa65df8bb..8a46c36c50dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6035,6 +6035,13 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt
 F:	drivers/gpu/drm/tiny/mi0283qt.c
 
+DRM DRIVER FOR MIPI DBI compatible panels
+M:	Noralf Trønnes <noralf@tronnes.org>
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
+F:	drivers/gpu/drm/panel/panel-mipi-dbi.c
+
 DRM DRIVER FOR MSM ADRENO GPU
 M:	Rob Clark <robdclark@gmail.com>
 M:	Sean Paul <sean@poorly.run>
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 434c2861bb40..1851cda5f877 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -274,6 +274,17 @@ config DRM_PANEL_LG_LG4573
 	  Say Y here if you want to enable support for LG4573 RGB panel.
 	  To compile this driver as a module, choose M here.
 
+config DRM_PANEL_MIPI_DBI
+	tristate "MIPI DBI compatible panel"
+	depends on SPI
+	depends on BACKLIGHT_CLASS_DEVICE
+	depends on DRM_KMS_HELPER
+	select DRM_KMS_CMA_HELPER
+	select DRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for MIPI DBI compatible panels.
+	  To compile this driver as a module, choose M here.
+
 config DRM_PANEL_NEC_NL8048HL11
 	tristate "NEC NL8048HL11 RGB panel"
 	depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d99fbbce49d1..a90c30459964 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 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_MIPI_DBI) += panel-mipi-dbi.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35950) += panel-novatek-nt35950.o
diff --git a/drivers/gpu/drm/panel/panel-mipi-dbi.c b/drivers/gpu/drm/panel/panel-mipi-dbi.c
new file mode 100644
index 000000000000..767658426061
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-mipi-dbi.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DRM driver for MIPI DBI compatible display panels
+ *
+ * Copyright 2022 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_modeset_helper.h>
+#include <video/mipi_display.h>
+
+static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 'B', 'I',
+					     0, 0, 0, 0, 0, 0, 0 };
+
+/*
+ * The display panel configuration is stored in a firmware file. The Device Tree 'model' property
+ * value with a '.bin' suffix is passed to request_firmware() to fetch this file.
+ */
+struct panel_mipi_dbi_config {
+	/* Magic string: panel_mipi_dbi_magic */
+	u8 magic[15];
+
+	/* Config file format version */
+	u8 file_format_version;
+
+	/* Width in pixels */
+	__be16 width;
+	/* Height in pixels */
+	__be16 height;
+
+	/* Width in millimeters (optional) */
+	__be16 width_mm;
+	/* Height in millimeters (optional) */
+	__be16 height_mm;
+
+	/* X-axis panel offset */
+	__be16 x_offset;
+	/* Y-axis panel offset */
+	__be16 y_offset;
+
+	/* 4 pad bytes, must be zero */
+	u8 pad[4];
+
+	/*
+	 * Optional MIPI commands to execute when the display pipeline is enabled.
+	 * This can be used to configure the display controller.
+	 *
+	 * The commands are stored in a byte array with the format:
+	 *     command, num_parameters, [ parameter, ...], command, ...
+	 *
+	 * Some commands require a pause before the next command can be received.
+	 * Inserting a delay in the command sequence is done by using the NOP command with one
+	 * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display
+	 * Command Set where it has no parameters).
+	 *
+	 * Example:
+	 *     command 0x11
+	 *     sleep 120ms
+	 *     command 0xb1 parameters 0x01, 0x2c, 0x2d
+	 *     command 0x29
+	 *
+	 * Byte sequence:
+	 *     0x11 0x00
+	 *     0x00 0x01 0x78
+	 *     0xb1 0x03 0x01 0x2c 0x2d
+	 *     0x29 0x00
+	 */
+	u8 commands[];
+};
+
+struct panel_mipi_dbi_commands {
+	const u8 *buf;
+	size_t len;
+};
+
+static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe,
+				  struct drm_crtc_state *crtc_state,
+				  struct drm_plane_state *plane_state)
+{
+	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+	struct panel_mipi_dbi_commands *commands = dbidev->driver_private;
+	struct mipi_dbi *dbi = &dbidev->dbi;
+	unsigned int i = 0;
+	int ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
+
+	drm_dbg(pipe->crtc.dev, "\n");
+
+	ret = mipi_dbi_poweron_conditional_reset(dbidev);
+	if (ret < 0)
+		goto out_exit;
+	if (ret == 1)
+		goto out_enable;
+
+	if (!commands)
+		goto out_enable;
+
+	while (i < commands->len) {
+		u8 command = commands->buf[i++];
+		u8 num_parameters = commands->buf[i++];
+		const u8 *parameters = &commands->buf[i];
+
+		if (command == 0x00 && num_parameters == 1)
+			msleep(parameters[0]);
+		else if (num_parameters)
+			mipi_dbi_command_stackbuf(dbi, command, parameters, num_parameters);
+		else
+			mipi_dbi_command(dbi, command);
+
+		i += num_parameters;
+	}
+
+out_enable:
+	mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+out_exit:
+	drm_dev_exit(idx);
+}
+
+static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = {
+	.enable = panel_mipi_dbi_enable,
+	.disable = mipi_dbi_pipe_disable,
+	.update = mipi_dbi_pipe_update,
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(panel_mipi_dbi_fops);
+
+static const struct drm_driver panel_mipi_dbi_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.fops			= &panel_mipi_dbi_fops,
+	DRM_GEM_CMA_DRIVER_OPS_VMAP,
+	.debugfs_init		= mipi_dbi_debugfs_init,
+	.name			= "panel-mipi-dbi",
+	.desc			= "MIPI DBI compatible display panel",
+	.date			= "20220103",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static int panel_mipi_dbi_parse_config(struct mipi_dbi_dev *dbidev,
+				       struct drm_display_mode *mode,
+				       const struct firmware *fw)
+{
+	const struct panel_mipi_dbi_config *config = (struct panel_mipi_dbi_config *)fw->data;
+	unsigned int width, height, x_offset, y_offset;
+	struct panel_mipi_dbi_commands *commands;
+	struct drm_device *drm = &dbidev->drm;
+	struct device *dev = dbidev->drm.dev;
+	size_t size = fw->size, commands_len;
+	unsigned int i = 0;
+
+	if (size < sizeof(*config)) {
+		dev_err(dev, "config: file size=%zu is too small\n", size);
+		return -EINVAL;
+	}
+
+	if (memcmp(config->magic, panel_mipi_dbi_magic, sizeof(config->magic))) {
+		dev_err(dev, "config: Bad magic: %15ph\n", config->magic);
+		return -EINVAL;
+	}
+
+	if (config->file_format_version != 1) {
+		dev_err(dev, "config: version=%u is not supported\n", config->file_format_version);
+		return -EINVAL;
+	}
+
+	width = be16_to_cpu(config->width);
+	height = be16_to_cpu(config->height);
+	x_offset = be16_to_cpu(config->x_offset);
+	y_offset = be16_to_cpu(config->y_offset);
+
+	drm_dbg(drm, "size=%zu version=%u\n", size, config->file_format_version);
+	drm_dbg(drm, "width=%u height=%u\n", width, height);
+	drm_dbg(drm, "x_offset=%u y_offset=%u\n", x_offset, y_offset);
+
+	if (width && height) {
+		struct drm_display_mode simple_mode = {
+			DRM_SIMPLE_MODE(width, height, be16_to_cpu(config->width_mm),
+					be16_to_cpu(config->height_mm))
+		};
+
+		*mode = simple_mode;
+	} else {
+		dev_err(dev, "config: width or height can't be zero\n");
+		return -EINVAL;
+	}
+
+	dbidev->left_offset = x_offset;
+	dbidev->top_offset = y_offset;
+
+	commands_len = size - sizeof(*config);
+	if (!commands_len)
+		return 0;
+
+	while ((i + 1) < commands_len) {
+		u8 command = config->commands[i++];
+		u8 num_parameters = config->commands[i++];
+		const u8 *parameters = &config->commands[i];
+
+		i += num_parameters;
+		if (i > commands_len) {
+			dev_err(dev, "config: command=0x%02x num_parameters=%u overflows\n",
+				command, num_parameters);
+			return -EINVAL;
+		}
+
+		if (command == 0x00 && num_parameters == 1)
+			drm_dbg(drm, "sleep %ums\n", parameters[0]);
+		else
+			drm_dbg(drm, "command %02x %*ph\n", command, num_parameters, parameters);
+	}
+
+	if (i != commands_len) {
+		dev_err(dev, "config: malformed command array\n");
+		return -EINVAL;
+	}
+
+	commands = devm_kzalloc(dev, sizeof(*commands), GFP_KERNEL);
+	if (!commands)
+		return -ENOMEM;
+
+	commands->len = commands_len;
+	commands->buf = devm_kmemdup(dev, config->commands, commands->len, GFP_KERNEL);
+	if (!commands->buf)
+		return -ENOMEM;
+
+	dbidev->driver_private = commands;
+
+	return 0;
+}
+
+static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct drm_display_mode mode;
+	struct mipi_dbi_dev *dbidev;
+	const struct firmware *fw;
+	struct drm_device *drm;
+	struct mipi_dbi *dbi;
+	struct gpio_desc *dc;
+	const char *model;
+	char fw_name[40];
+	int ret;
+
+	ret = device_property_read_string(dev, "model", &model);
+	if (ret) {
+		dev_err(dev, "Failed to get 'model' property\n");
+		return ret;
+	}
+
+	dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm);
+	if (IS_ERR(dbidev))
+		return PTR_ERR(dbidev);
+
+	dbi = &dbidev->dbi;
+	drm = &dbidev->drm;
+
+	snprintf(fw_name, sizeof(fw_name), "%s.bin", model);
+
+	ret = request_firmware(&fw, fw_name, dev);
+	if (ret)
+		return ret;
+
+	ret = panel_mipi_dbi_parse_config(dbidev, &mode, fw);
+	release_firmware(fw);
+	if (ret)
+		return ret;
+
+	dbidev->regulator = devm_regulator_get(dev, "power");
+	if (IS_ERR(dbidev->regulator))
+		return dev_err_probe(dev, PTR_ERR(dbidev->regulator),
+				     "Failed to get regulator 'power'\n");
+
+	dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(dbi->reset))
+		return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n");
+
+	dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc))
+		return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n");
+
+	dbidev->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(dbidev->backlight))
+		return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n");
+
+	ret = mipi_dbi_spi_init(spi, dbi, dc);
+	if (ret)
+		return ret;
+
+	if (device_property_present(dev, "write-only"))
+		dbi->read_commands = NULL;
+
+	ret = mipi_dbi_dev_init(dbidev, &panel_mipi_dbi_pipe_funcs, &mode, 0);
+	if (ret)
+		return ret;
+
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 0);
+
+	return 0;
+}
+
+static int panel_mipi_dbi_spi_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
+}
+
+static void panel_mipi_dbi_spi_shutdown(struct spi_device *spi)
+{
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
+}
+
+static int __maybe_unused panel_mipi_dbi_pm_suspend(struct device *dev)
+{
+	return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
+}
+
+static int __maybe_unused panel_mipi_dbi_pm_resume(struct device *dev)
+{
+	drm_mode_config_helper_resume(dev_get_drvdata(dev));
+
+	return 0;
+}
+
+static const struct dev_pm_ops panel_mipi_dbi_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(panel_mipi_dbi_pm_suspend, panel_mipi_dbi_pm_resume)
+};
+
+static const struct of_device_id panel_mipi_dbi_spi_of_match[] = {
+	{ .compatible = "panel-mipi-dbi-spi" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, panel_mipi_dbi_spi_of_match);
+
+static const struct spi_device_id panel_mipi_dbi_spi_id[] = {
+	{ "panel-mipi-dbi-spi", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, panel_mipi_dbi_spi_id);
+
+static struct spi_driver panel_mipi_dbi_spi_driver = {
+	.driver = {
+		.name = "panel-mipi-dbi-spi",
+		.owner = THIS_MODULE,
+		.of_match_table = panel_mipi_dbi_spi_of_match,
+		.pm = &panel_mipi_dbi_pm_ops,
+	},
+	.id_table = panel_mipi_dbi_spi_id,
+	.probe = panel_mipi_dbi_spi_probe,
+	.remove = panel_mipi_dbi_spi_remove,
+	.shutdown = panel_mipi_dbi_spi_shutdown,
+};
+module_spi_driver(panel_mipi_dbi_spi_driver);
+
+MODULE_DESCRIPTION("MIPI DBI compatible display panel driver");
+MODULE_AUTHOR("Noralf Trønnes");
+MODULE_LICENSE("GPL");
-- 
2.33.0

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4081 bytes --]

On Sun, 23 Jan 2022 18:25:18 +0100, Noralf Trønnes wrote:
> Add binding for MIPI DBI compatible SPI panels.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
schemas/display/panel/panel/panel-common.yaml: ignoring, error parsing file
make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml'
schemas/display/panel/panel/panel-common.yaml: ignoring, error parsing file
Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 170, in <module>
    sg.check_trees(filename, testtree)
  File "/usr/local/bin/dt-validate", line 119, in check_trees
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 105, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 49, in check_node
    errors = sorted(dtschema.DTValidator(schema).iter_errors(node), key=lambda e: e.linecol)
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 362, in allOf
    yield from validator.descend(instance, subschema, schema_path=index)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 298, in ref
    yield from validator.descend(instance, resolved)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 219, in iter_errors
    scope = id_of(_schema)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 96, in _id_of
    return schema.get("$id", "")
AttributeError: 'NoneType' object has no attribute 'get'
make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1583159

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4081 bytes --]

On Sun, 23 Jan 2022 18:25:18 +0100, Noralf Trønnes wrote:
> Add binding for MIPI DBI compatible SPI panels.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
schemas/display/panel/panel/panel-common.yaml: ignoring, error parsing file
make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml'
schemas/display/panel/panel/panel-common.yaml: ignoring, error parsing file
Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 170, in <module>
    sg.check_trees(filename, testtree)
  File "/usr/local/bin/dt-validate", line 119, in check_trees
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 105, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 49, in check_node
    errors = sorted(dtschema.DTValidator(schema).iter_errors(node), key=lambda e: e.linecol)
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 362, in allOf
    yield from validator.descend(instance, subschema, schema_path=index)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 298, in ref
    yield from validator.descend(instance, resolved)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 219, in iter_errors
    scope = id_of(_schema)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 96, in _id_of
    return schema.get("$id", "")
AttributeError: 'NoneType' object has no attribute 'get'
make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1583159

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Add binding for MIPI DBI compatible SPI panels.

I'm sure we already have MIPI DBI panels. What's this for?

>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> new file mode 100644
> index 000000000000..d6c8accb045c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MIPI DBI SPI Panels Device Tree Bindings
> +
> +maintainers:
> +  - Noralf Trønnes <noralf@tronnes.org>
> +
> +description:
> +  This binding is for display panels using a MIPI DBI controller
> +  in SPI mode.
> +
> +allOf:
> +  - $ref: panel/panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: panel-mipi-dbi-spi

Does the MIPI spec define how to power on a DBI panel with regulators,
enable/reset lines, etc. and all the timing constraints between those?
If not, then this compatible on its own is useless. It's fine as a
fallback if it's presence means the panel uses only standard DBI
commands and no vendor specific commands.

> +
> +  model:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: The name of the display panel.
> +
> +  write-only:
> +    type: boolean
> +    description:
> +      Controller is not readable (ie. MISO is not wired up).
> +
> +  dc-gpios:
> +    maxItems: 1
> +    description: |
> +      Controller data/command selection (D/CX) in 4-line SPI mode.
> +      If not set, the controller is in 3-line SPI mode.
> +
> +  backlight: true
> +  reg: true
> +  reset-gpios: true
> +
> +required:
> +  - compatible
> +  - model
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            display@0{
> +                    compatible = "panel-mipi-dbi-spi";
> +                    model = "sainsmart18";
> +                    reg = <0>;
> +                    spi-max-frequency = <40000000>;
> +                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> +                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
> +                    write-only;
> +            };
> +    };
> +
> +...
> --
> 2.33.0
>

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Add binding for MIPI DBI compatible SPI panels.

I'm sure we already have MIPI DBI panels. What's this for?

>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> new file mode 100644
> index 000000000000..d6c8accb045c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MIPI DBI SPI Panels Device Tree Bindings
> +
> +maintainers:
> +  - Noralf Trønnes <noralf@tronnes.org>
> +
> +description:
> +  This binding is for display panels using a MIPI DBI controller
> +  in SPI mode.
> +
> +allOf:
> +  - $ref: panel/panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: panel-mipi-dbi-spi

Does the MIPI spec define how to power on a DBI panel with regulators,
enable/reset lines, etc. and all the timing constraints between those?
If not, then this compatible on its own is useless. It's fine as a
fallback if it's presence means the panel uses only standard DBI
commands and no vendor specific commands.

> +
> +  model:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: The name of the display panel.
> +
> +  write-only:
> +    type: boolean
> +    description:
> +      Controller is not readable (ie. MISO is not wired up).
> +
> +  dc-gpios:
> +    maxItems: 1
> +    description: |
> +      Controller data/command selection (D/CX) in 4-line SPI mode.
> +      If not set, the controller is in 3-line SPI mode.
> +
> +  backlight: true
> +  reg: true
> +  reset-gpios: true
> +
> +required:
> +  - compatible
> +  - model
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            display@0{
> +                    compatible = "panel-mipi-dbi-spi";
> +                    model = "sainsmart18";
> +                    reg = <0>;
> +                    spi-max-frequency = <40000000>;
> +                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> +                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
> +                    write-only;
> +            };
> +    };
> +
> +...
> --
> 2.33.0
>

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Den 24.01.2022 17.08, skrev Rob Herring:
> On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> Add binding for MIPI DBI compatible SPI panels.
> 
> I'm sure we already have MIPI DBI panels. What's this for?
> 

It aims to use one driver to cover all MIPI DBI panels where the
controller setup is loaded from userspace in a firmware file. The cover
letter points to the discussion where Maxime proposed this:

https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/

Noralf.

>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>> new file mode 100644
>> index 000000000000..d6c8accb045c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>> @@ -0,0 +1,69 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MIPI DBI SPI Panels Device Tree Bindings
>> +
>> +maintainers:
>> +  - Noralf Trønnes <noralf@tronnes.org>
>> +
>> +description:
>> +  This binding is for display panels using a MIPI DBI controller
>> +  in SPI mode.
>> +
>> +allOf:
>> +  - $ref: panel/panel-common.yaml#
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: panel-mipi-dbi-spi
> 
> Does the MIPI spec define how to power on a DBI panel with regulators,
> enable/reset lines, etc. and all the timing constraints between those?
> If not, then this compatible on its own is useless. It's fine as a
> fallback if it's presence means the panel uses only standard DBI
> commands and no vendor specific commands.
> 
>> +
>> +  model:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: The name of the display panel.
>> +
>> +  write-only:
>> +    type: boolean
>> +    description:
>> +      Controller is not readable (ie. MISO is not wired up).
>> +
>> +  dc-gpios:
>> +    maxItems: 1
>> +    description: |
>> +      Controller data/command selection (D/CX) in 4-line SPI mode.
>> +      If not set, the controller is in 3-line SPI mode.
>> +
>> +  backlight: true
>> +  reg: true
>> +  reset-gpios: true
>> +
>> +required:
>> +  - compatible
>> +  - model
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    spi {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            display@0{
>> +                    compatible = "panel-mipi-dbi-spi";
>> +                    model = "sainsmart18";
>> +                    reg = <0>;
>> +                    spi-max-frequency = <40000000>;
>> +                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
>> +                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
>> +                    write-only;
>> +            };
>> +    };
>> +
>> +...
>> --
>> 2.33.0
>>

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Den 24.01.2022 17.08, skrev Rob Herring:
> On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> Add binding for MIPI DBI compatible SPI panels.
> 
> I'm sure we already have MIPI DBI panels. What's this for?
> 

It aims to use one driver to cover all MIPI DBI panels where the
controller setup is loaded from userspace in a firmware file. The cover
letter points to the discussion where Maxime proposed this:

https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/

Noralf.

>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>> new file mode 100644
>> index 000000000000..d6c8accb045c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>> @@ -0,0 +1,69 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MIPI DBI SPI Panels Device Tree Bindings
>> +
>> +maintainers:
>> +  - Noralf Trønnes <noralf@tronnes.org>
>> +
>> +description:
>> +  This binding is for display panels using a MIPI DBI controller
>> +  in SPI mode.
>> +
>> +allOf:
>> +  - $ref: panel/panel-common.yaml#
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: panel-mipi-dbi-spi
> 
> Does the MIPI spec define how to power on a DBI panel with regulators,
> enable/reset lines, etc. and all the timing constraints between those?
> If not, then this compatible on its own is useless. It's fine as a
> fallback if it's presence means the panel uses only standard DBI
> commands and no vendor specific commands.
> 
>> +
>> +  model:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: The name of the display panel.
>> +
>> +  write-only:
>> +    type: boolean
>> +    description:
>> +      Controller is not readable (ie. MISO is not wired up).
>> +
>> +  dc-gpios:
>> +    maxItems: 1
>> +    description: |
>> +      Controller data/command selection (D/CX) in 4-line SPI mode.
>> +      If not set, the controller is in 3-line SPI mode.
>> +
>> +  backlight: true
>> +  reg: true
>> +  reset-gpios: true
>> +
>> +required:
>> +  - compatible
>> +  - model
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    spi {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            display@0{
>> +                    compatible = "panel-mipi-dbi-spi";
>> +                    model = "sainsmart18";
>> +                    reg = <0>;
>> +                    spi-max-frequency = <40000000>;
>> +                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
>> +                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
>> +                    write-only;
>> +            };
>> +    };
>> +
>> +...
>> --
>> 2.33.0
>>

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>
>
> Den 24.01.2022 17.08, skrev Rob Herring:
> > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> >>
> >> Add binding for MIPI DBI compatible SPI panels.
> >
> > I'm sure we already have MIPI DBI panels. What's this for?
> >
>
> It aims to use one driver to cover all MIPI DBI panels where the
> controller setup is loaded from userspace in a firmware file.

What's the solution when the user wants a splash screen in the
bootloader and also wants multiple panels supported?

Also, 1 driver doesn't dictate 1 compatible. A one to many
relationship is fine and makes the decision entirely the OS's.

> The cover
> letter points to the discussion where Maxime proposed this:
>
> https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/

The proposal there is:

> compatible = "panel-spi";
> model = "panel-from-random-place-42";

The same thing can be accomplished with this:

compatible = "panel-from-random-place-42", "panel-spi";

What's the advantage of hijacking 'model'?

Rob

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>
>
> Den 24.01.2022 17.08, skrev Rob Herring:
> > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> >>
> >> Add binding for MIPI DBI compatible SPI panels.
> >
> > I'm sure we already have MIPI DBI panels. What's this for?
> >
>
> It aims to use one driver to cover all MIPI DBI panels where the
> controller setup is loaded from userspace in a firmware file.

What's the solution when the user wants a splash screen in the
bootloader and also wants multiple panels supported?

Also, 1 driver doesn't dictate 1 compatible. A one to many
relationship is fine and makes the decision entirely the OS's.

> The cover
> letter points to the discussion where Maxime proposed this:
>
> https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/

The proposal there is:

> compatible = "panel-spi";
> model = "panel-from-random-place-42";

The same thing can be accomplished with this:

compatible = "panel-from-random-place-42", "panel-spi";

What's the advantage of hijacking 'model'?

Rob

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Den 24.01.2022 17.42, skrev Rob Herring:
> On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>>
>>
>> Den 24.01.2022 17.08, skrev Rob Herring:
>>> On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>>>
>>>> Add binding for MIPI DBI compatible SPI panels.
>>>
>>> I'm sure we already have MIPI DBI panels. What's this for?
>>>
>>
>> It aims to use one driver to cover all MIPI DBI panels where the
>> controller setup is loaded from userspace in a firmware file.
> 
> What's the solution when the user wants a splash screen in the
> bootloader and also wants multiple panels supported?
> 

In that case the bootloader needs builtin support for the screen. The OS
driver can then detect that the controller is configured and just skip
the configure stage. I have yet to see someone implement suppport for
such a SPI panel in a bootloader though.

> Also, 1 driver doesn't dictate 1 compatible. A one to many
> relationship is fine and makes the decision entirely the OS's.
> 
>> The cover
>> letter points to the discussion where Maxime proposed this:
>>
>> https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> 
> The proposal there is:
> 
>> compatible = "panel-spi";
>> model = "panel-from-random-place-42";
> 
> The same thing can be accomplished with this:
> 
> compatible = "panel-from-random-place-42", "panel-spi";
> 
> What's the advantage of hijacking 'model'?
> 

That's what Maxime proposed, but having it in the compatible like this
is even better. I suppose it's ok for the driver to use the compatible
to load the firmware file?

Noralf.

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Den 24.01.2022 17.42, skrev Rob Herring:
> On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>>
>>
>> Den 24.01.2022 17.08, skrev Rob Herring:
>>> On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>>>
>>>> Add binding for MIPI DBI compatible SPI panels.
>>>
>>> I'm sure we already have MIPI DBI panels. What's this for?
>>>
>>
>> It aims to use one driver to cover all MIPI DBI panels where the
>> controller setup is loaded from userspace in a firmware file.
> 
> What's the solution when the user wants a splash screen in the
> bootloader and also wants multiple panels supported?
> 

In that case the bootloader needs builtin support for the screen. The OS
driver can then detect that the controller is configured and just skip
the configure stage. I have yet to see someone implement suppport for
such a SPI panel in a bootloader though.

> Also, 1 driver doesn't dictate 1 compatible. A one to many
> relationship is fine and makes the decision entirely the OS's.
> 
>> The cover
>> letter points to the discussion where Maxime proposed this:
>>
>> https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> 
> The proposal there is:
> 
>> compatible = "panel-spi";
>> model = "panel-from-random-place-42";
> 
> The same thing can be accomplished with this:
> 
> compatible = "panel-from-random-place-42", "panel-spi";
> 
> What's the advantage of hijacking 'model'?
> 

That's what Maxime proposed, but having it in the compatible like this
is even better. I suppose it's ok for the driver to use the compatible
to load the firmware file?

Noralf.

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

Hi Rob,

On Mon, Jan 24, 2022 at 10:42:37AM -0600, Rob Herring wrote:
> On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> >
> >
> >
> > Den 24.01.2022 17.08, skrev Rob Herring:
> > > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > >>
> > >> Add binding for MIPI DBI compatible SPI panels.
> > >
> > > I'm sure we already have MIPI DBI panels. What's this for?
> > >
> >
> > It aims to use one driver to cover all MIPI DBI panels where the
> > controller setup is loaded from userspace in a firmware file.
> 
> What's the solution when the user wants a splash screen in the
> bootloader and also wants multiple panels supported?
> 
> Also, 1 driver doesn't dictate 1 compatible. A one to many
> relationship is fine and makes the decision entirely the OS's.
> 
> > The cover
> > letter points to the discussion where Maxime proposed this:
> >
> > https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> 
> The proposal there is:
> 
> > compatible = "panel-spi";
> > model = "panel-from-random-place-42";
> 
> The same thing can be accomplished with this:
> 
> compatible = "panel-from-random-place-42", "panel-spi";
> 
> What's the advantage of hijacking 'model'?

So, the main issue is that a panel is essentially two things: a
controller and the actual panel.

The controller has an initialization sequence of its own, and part of it
is parameters to match the panel.

So you can have identical controllers that won't have the same
initialization sequence because they don't have the same panel.

I was assuming that a compatible would be more about the controller, so
we needed something else, thus "model"

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

Hi Rob,

On Mon, Jan 24, 2022 at 10:42:37AM -0600, Rob Herring wrote:
> On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> >
> >
> >
> > Den 24.01.2022 17.08, skrev Rob Herring:
> > > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > >>
> > >> Add binding for MIPI DBI compatible SPI panels.
> > >
> > > I'm sure we already have MIPI DBI panels. What's this for?
> > >
> >
> > It aims to use one driver to cover all MIPI DBI panels where the
> > controller setup is loaded from userspace in a firmware file.
> 
> What's the solution when the user wants a splash screen in the
> bootloader and also wants multiple panels supported?
> 
> Also, 1 driver doesn't dictate 1 compatible. A one to many
> relationship is fine and makes the decision entirely the OS's.
> 
> > The cover
> > letter points to the discussion where Maxime proposed this:
> >
> > https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> 
> The proposal there is:
> 
> > compatible = "panel-spi";
> > model = "panel-from-random-place-42";
> 
> The same thing can be accomplished with this:
> 
> compatible = "panel-from-random-place-42", "panel-spi";
> 
> What's the advantage of hijacking 'model'?

So, the main issue is that a panel is essentially two things: a
controller and the actual panel.

The controller has an initialization sequence of its own, and part of it
is parameters to match the panel.

So you can have identical controllers that won't have the same
initialization sequence because they don't have the same panel.

I was assuming that a compatible would be more about the controller, so
we needed something else, thus "model"

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Thu, Jan 27, 2022 at 3:36 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Rob,
>
> On Mon, Jan 24, 2022 at 10:42:37AM -0600, Rob Herring wrote:
> > On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > >
> > >
> > >
> > > Den 24.01.2022 17.08, skrev Rob Herring:
> > > > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > > >>
> > > >> Add binding for MIPI DBI compatible SPI panels.
> > > >
> > > > I'm sure we already have MIPI DBI panels. What's this for?
> > > >
> > >
> > > It aims to use one driver to cover all MIPI DBI panels where the
> > > controller setup is loaded from userspace in a firmware file.
> >
> > What's the solution when the user wants a splash screen in the
> > bootloader and also wants multiple panels supported?
> >
> > Also, 1 driver doesn't dictate 1 compatible. A one to many
> > relationship is fine and makes the decision entirely the OS's.
> >
> > > The cover
> > > letter points to the discussion where Maxime proposed this:
> > >
> > > https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> >
> > The proposal there is:
> >
> > > compatible = "panel-spi";
> > > model = "panel-from-random-place-42";
> >
> > The same thing can be accomplished with this:
> >
> > compatible = "panel-from-random-place-42", "panel-spi";
> >
> > What's the advantage of hijacking 'model'?
>
> So, the main issue is that a panel is essentially two things: a
> controller and the actual panel.
>
> The controller has an initialization sequence of its own, and part of it
> is parameters to match the panel.
>
> So you can have identical controllers that won't have the same
> initialization sequence because they don't have the same panel.
>
> I was assuming that a compatible would be more about the controller, so
> we needed something else, thus "model"

We already have cases like that which have "foo,some-panel", "bar,a-ctrlr-ic".

Rob

Re: [PATCH 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Thu, Jan 27, 2022 at 3:36 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Rob,
>
> On Mon, Jan 24, 2022 at 10:42:37AM -0600, Rob Herring wrote:
> > On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > >
> > >
> > >
> > > Den 24.01.2022 17.08, skrev Rob Herring:
> > > > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > > >>
> > > >> Add binding for MIPI DBI compatible SPI panels.
> > > >
> > > > I'm sure we already have MIPI DBI panels. What's this for?
> > > >
> > >
> > > It aims to use one driver to cover all MIPI DBI panels where the
> > > controller setup is loaded from userspace in a firmware file.
> >
> > What's the solution when the user wants a splash screen in the
> > bootloader and also wants multiple panels supported?
> >
> > Also, 1 driver doesn't dictate 1 compatible. A one to many
> > relationship is fine and makes the decision entirely the OS's.
> >
> > > The cover
> > > letter points to the discussion where Maxime proposed this:
> > >
> > > https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> >
> > The proposal there is:
> >
> > > compatible = "panel-spi";
> > > model = "panel-from-random-place-42";
> >
> > The same thing can be accomplished with this:
> >
> > compatible = "panel-from-random-place-42", "panel-spi";
> >
> > What's the advantage of hijacking 'model'?
>
> So, the main issue is that a panel is essentially two things: a
> controller and the actual panel.
>
> The controller has an initialization sequence of its own, and part of it
> is parameters to match the panel.
>
> So you can have identical controllers that won't have the same
> initialization sequence because they don't have the same panel.
>
> I was assuming that a compatible would be more about the controller, so
> we needed something else, thus "model"

We already have cases like that which have "foo,some-panel", "bar,a-ctrlr-ic".

Rob