Hi,
Here's a follow-up on the previous discussion about the current state of
DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
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.
This patchset introduces the following:
* Patch [2/6] slightly tweaks the MIPI DSI code so that it now also
supports MIPI DBI over various hardware buses. Because if you think
about it, DSI is just the same as DBI just on a different hardware
bus. The DSI host drivers, now DSI/DBI host drivers, set compatibility
bits for the hardware buses they support (DSI, DBI/i8080, DBI/SPI9,
DBI/SPI+GPIO), which is matched against the hardware bus that panel
drivers request.
* For the DBI panels connected over SPI, a new DSI/DBI host driver is
added in patch [3/6]. It allows MIPI DBI panel drivers to be written
with the DSI/DBI framework, even if they are connected over SPI,
instead of registering as SPI device drivers. Since most of these
panels can be connected over various buses, it permits to reuse the
same driver independently of the bus used.
* Patch [4/6] adds a panel driver for NewVision NV3052C based panels.
This has been successfully tested on the Anbernic RG350M handheld
console, along with the dbi-spi bridge and the ingenic-drm driver.
* A TinyDRM driver for DSI/DBI panels, once again independent of the bus
used; the only dependency (currently) being that the panel must
understand DCS commands.
* A DRM panel driver to test the stack. This driver controls Ilitek
ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
TFT LCD panel. This panel was converted from
drivers/gpu/drm/tiny/ili9341.c.
Patches [1-4] were successfully tested on hardware.
Patches [5-6] were compile-tested and runtime-tested but without
hardware connected, so I'd want a Tested-by on these two.
Another thing to note, is that it does not break Device Tree ABI. The
display node stays the same:
spi {
...
display@0 {
compatible = "adafruit,yx240qv29", "ilitek,ili9341";
reg = <0>;
spi-max-frequency = <32000000>;
dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
rotation = <270>;
backlight = <&backlight>;
};
};
The reason it works, is that the "adafruit,yx240qv29" device is probed
on the SPI bus, so it will match with the SPI/DBI host driver. This will
in turn register the very same node with the DSI bus, and the ILI9341
DRM panel driver will probe. The driver will detect that no controller
is linked to the panel, and eventually register the DBI/DSI TinyDRM
driver.
One drawback of this approach, is that the "adafruit,yx240qv29" must be
added to the dbi-spi bridge driver (unless a custom rule is added for a
"dbi-spi" fallback compatible string). I still think that it's a small
price to pay to avoid breaking the Device Tree bindings.
Feedback welcome.
Cheers,
-Paul
Paul Cercueil (6):
dt-bindings: display: Document NewVision NV3052C DT node
drm: dsi: Let host and device specify supported bus
drm/bridge: Add SPI DBI host driver
drm/panel: Add panel driver for NewVision NV3052C based LCDs
drm/tiny: Add TinyDRM for DSI/DBI panels
drm/panel: Add Ilitek ILI9341 DBI panel driver
.../display/panel/newvision,nv3052c.yaml | 69 +++
drivers/gpu/drm/bridge/Kconfig | 8 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/dbi-spi.c | 262 +++++++++
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 | 345 ++++++++++++
.../gpu/drm/panel/panel-newvision-nv3052c.c | 523 ++++++++++++++++++
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 | 31 ++
13 files changed, 1543 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
create mode 100644 drivers/gpu/drm/bridge/dbi-spi.c
create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c
create mode 100644 drivers/gpu/drm/panel/panel-newvision-nv3052c.c
create mode 100644 drivers/gpu/drm/tiny/tiny-dsi.c
--
2.27.0
Add documentation for the Device Tree node for LCD panels based on the
NewVision NV3052C controller.
Signed-off-by: Paul Cercueil <[email protected]>
---
.../display/panel/newvision,nv3052c.yaml | 69 +++++++++++++++++++
1 file changed, 69 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..751a28800fc2
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
@@ -0,0 +1,69 @@
+# 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 <[email protected]>
+
+description: |
+ This is a driver for 320x240 TFT panels, accepting a variety of input
+ streams that get adapted and scaled to the panel. The panel output has
+ 960 TFT source driver pins and 240 TFT gate driver pins, VCOM, VCOML and
+ VCOMH outputs.
+
+ 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
+ port: true
+
+required:
+ - compatible
+ - reg
+
+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.27.0
This driver is for the Ilitek ILI9341 based YX240QV29-T 2.4" 240x320 TFT
LCD panel from Adafruit.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 345 +++++++++++++++++++
3 files changed, 355 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 6a8a51a702d8..132a42a81895 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 a0516ced87db..ad5e6089c3ef 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..3af6628639d6
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -0,0 +1,345 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <[email protected]>
+ * Copyright 2020 Paul Cercueil <[email protected]>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/dma-buf.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/property.h>
+#include <drm/drm_atomic_helper.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <video/mipi_display.h>
+
+#define ILI9341_FRMCTR1 0xb1
+#define ILI9341_DISCTRL 0xb6
+#define ILI9341_ETMOD 0xb7
+
+#define ILI9341_PWCTRL1 0xc0
+#define ILI9341_PWCTRL2 0xc1
+#define ILI9341_VMCTRL1 0xc5
+#define ILI9341_VMCTRL2 0xc7
+#define ILI9341_PWCTRLA 0xcb
+#define ILI9341_PWCTRLB 0xcf
+
+#define ILI9341_PGAMCTRL 0xe0
+#define ILI9341_NGAMCTRL 0xe1
+#define ILI9341_DTCTRLA 0xe8
+#define ILI9341_DTCTRLB 0xea
+#define ILI9341_PWRSEQ 0xed
+
+#define ILI9341_EN3GAM 0xf2
+#define ILI9341_PUMPCTRL 0xf7
+
+#define ILI9341_MADCTL_BGR BIT(3)
+#define ILI9341_MADCTL_MV BIT(5)
+#define ILI9341_MADCTL_MX BIT(6)
+#define ILI9341_MADCTL_MY BIT(7)
+
+struct ili9341_pdata {
+ struct drm_display_mode mode;
+ unsigned int width_mm;
+ unsigned int height_mm;
+ unsigned int bus_type;
+ unsigned int lanes;
+};
+
+struct ili9341 {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ const struct ili9341_pdata *pdata;
+
+ struct gpio_desc *reset_gpiod;
+ struct backlight_device *backlight;
+ u32 rotation;
+};
+
+#define mipi_dcs_command(dsi, cmd, seq...) \
+({ \
+ u8 d[] = { seq }; \
+ mipi_dsi_dcs_write(dsi, cmd, d, ARRAY_SIZE(d)); \
+})
+
+static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel)
+{
+ return container_of(panel, struct ili9341, panel);
+}
+
+static int ili9341_prepare(struct drm_panel *panel)
+{
+ struct ili9341 *priv = panel_to_ili9341(panel);
+ struct mipi_dsi_device *dsi = priv->dsi;
+ u8 addr_mode;
+ int ret;
+
+ gpiod_set_value_cansleep(priv->reset_gpiod, 0);
+ usleep_range(20, 1000);
+ gpiod_set_value_cansleep(priv->reset_gpiod, 1);
+ msleep(120);
+
+ ret = mipi_dcs_command(dsi, MIPI_DCS_SOFT_RESET);
+ if (ret) {
+ dev_err(panel->dev, "Failed to send reset command: %d\n", ret);
+ return ret;
+ }
+
+ /* Wait 5ms after soft reset per MIPI DCS spec */
+ usleep_range(5000, 20000);
+
+ mipi_dcs_command(dsi, MIPI_DCS_SET_DISPLAY_OFF);
+
+ mipi_dcs_command(dsi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+ mipi_dcs_command(dsi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+ mipi_dcs_command(dsi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+ mipi_dcs_command(dsi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+ mipi_dcs_command(dsi, ILI9341_PUMPCTRL, 0x20);
+ mipi_dcs_command(dsi, ILI9341_DTCTRLB, 0x00, 0x00);
+
+ /* Power Control */
+ mipi_dcs_command(dsi, ILI9341_PWCTRL1, 0x23);
+ mipi_dcs_command(dsi, ILI9341_PWCTRL2, 0x10);
+ /* VCOM */
+ mipi_dcs_command(dsi, ILI9341_VMCTRL1, 0x3e, 0x28);
+ mipi_dcs_command(dsi, ILI9341_VMCTRL2, 0x86);
+
+ /* Memory Access Control */
+ mipi_dcs_command(dsi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
+
+ /* Frame Rate */
+ mipi_dcs_command(dsi, ILI9341_FRMCTR1, 0x00, 0x1b);
+
+ /* Gamma */
+ mipi_dcs_command(dsi, ILI9341_EN3GAM, 0x00);
+ mipi_dcs_command(dsi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
+ mipi_dcs_command(dsi, ILI9341_PGAMCTRL,
+ 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
+ 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
+ mipi_dcs_command(dsi, ILI9341_NGAMCTRL,
+ 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
+ 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
+
+ /* DDRAM */
+ mipi_dcs_command(dsi, ILI9341_ETMOD, 0x07);
+
+ /* Display */
+ mipi_dcs_command(dsi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
+ mipi_dcs_command(dsi, MIPI_DCS_EXIT_SLEEP_MODE);
+ msleep(100);
+
+ mipi_dcs_command(dsi, MIPI_DCS_SET_DISPLAY_ON);
+ msleep(100);
+
+ switch (priv->rotation) {
+ default:
+ addr_mode = ILI9341_MADCTL_MX;
+ break;
+ case 90:
+ addr_mode = ILI9341_MADCTL_MV;
+ break;
+ case 180:
+ addr_mode = ILI9341_MADCTL_MY;
+ break;
+ case 270:
+ addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
+ ILI9341_MADCTL_MX;
+ break;
+ }
+ addr_mode |= ILI9341_MADCTL_BGR;
+ mipi_dcs_command(dsi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+
+ return 0;
+}
+
+static int ili9341_enable(struct drm_panel *panel)
+{
+ struct ili9341 *priv = panel_to_ili9341(panel);
+
+ backlight_enable(priv->backlight);
+
+ return 0;
+}
+
+static int ili9341_disable(struct drm_panel *panel)
+{
+ struct ili9341 *priv = panel_to_ili9341(panel);
+
+ backlight_disable(priv->backlight);
+
+ return 0;
+}
+
+static int ili9341_unprepare(struct drm_panel *panel)
+{
+ struct ili9341 *priv = panel_to_ili9341(panel);
+
+ mipi_dcs_command(priv->dsi, MIPI_DCS_SET_DISPLAY_OFF);
+
+ return 0;
+}
+
+static int ili9341_get_modes(struct drm_panel *panel,
+ struct drm_connector *connector)
+{
+ struct ili9341 *priv = panel_to_ili9341(panel);
+ struct drm_display_mode *mode;
+ u32 format = MEDIA_BUS_FMT_RGB565_1X16;
+
+ mode = drm_mode_duplicate(connector->dev, &priv->pdata->mode);
+ if (!mode) {
+ dev_err(panel->dev, "failed to add mode %ux%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,
+ .enable = ili9341_enable,
+ .disable = ili9341_disable,
+ .get_modes = ili9341_get_modes,
+};
+
+static int ili9341_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct ili9341 *priv;
+ int ret;
+
+ /* See comment for mipi_dbi_spi_init() */
+ if (!dev->coherent_dma_mask) {
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ if (ret) {
+ dev_warn(dev, "Failed to set dma mask %d\n", ret);
+ return ret;
+ }
+ }
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mipi_dsi_set_drvdata(dsi, priv);
+ priv->dsi = dsi;
+
+ device_property_read_u32(dev, "rotation", &priv->rotation);
+
+ priv->pdata = device_get_match_data(dev);
+ if (!priv->pdata)
+ return -EINVAL;
+
+ drm_panel_init(&priv->panel, dev, &ili9341_funcs,
+ DRM_MODE_CONNECTOR_DPI);
+
+ priv->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset_gpiod)) {
+ dev_err(dev, "Couldn't get our reset GPIO\n");
+ return PTR_ERR(priv->reset_gpiod);
+ }
+
+ priv->backlight = devm_of_find_backlight(dev);
+ if (IS_ERR(priv->backlight)) {
+ ret = PTR_ERR(priv->backlight);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get backlight handle\n");
+ return ret;
+ }
+
+ ret = drm_panel_add(&priv->panel);
+ if (ret < 0) {
+ dev_err(dev, "Failed to register panel\n");
+ return ret;
+ }
+
+ dsi->bus_type = priv->pdata->bus_type;
+ dsi->lanes = priv->pdata->lanes;
+ dsi->format = MIPI_DSI_FMT_RGB565;
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret) {
+ dev_err(dev, "Failed to attach DSI panel\n");
+ goto err_panel_remove;
+ }
+
+ ret = mipi_dsi_maybe_register_tiny_driver(dsi);
+ if (ret) {
+ dev_err(dev, "Failed to init TinyDRM driver\n");
+ goto err_mipi_dsi_detach;
+ }
+
+ return 0;
+
+err_mipi_dsi_detach:
+ mipi_dsi_detach(dsi);
+err_panel_remove:
+ drm_panel_remove(&priv->panel);
+ return ret;
+}
+
+static int ili9341_remove(struct mipi_dsi_device *dsi)
+{
+ struct ili9341 *priv = mipi_dsi_get_drvdata(dsi);
+
+ mipi_dsi_detach(dsi);
+ drm_panel_remove(&priv->panel);
+
+ if (priv->backlight)
+ put_device(&priv->backlight->dev);
+
+ return 0;
+}
+
+static const struct ili9341_pdata yx240qv29_pdata = {
+ .mode = { DRM_SIMPLE_MODE(240, 320, 37, 49) },
+ .width_mm = 0, // TODO
+ .height_mm = 0, // TODO
+ .bus_type = MIPI_DEVICE_TYPE_DBI_SPI_MODE3,
+ .lanes = 1,
+};
+
+static const struct of_device_id ili9341_of_match[] = {
+ { .compatible = "adafruit,yx240qv29", .data = &yx240qv29_pdata },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ili9341_of_match);
+
+static struct mipi_dsi_driver ili9341_dsi_driver = {
+ .probe = ili9341_probe,
+ .remove = ili9341_remove,
+ .driver = {
+ .name = "ili9341-dsi",
+ .of_match_table = ili9341_of_match,
+ },
+};
+module_mipi_dsi_driver(ili9341_dsi_driver);
+
+MODULE_DESCRIPTION("Ilitek ILI9341 DRM panel driver");
+MODULE_AUTHOR("David Lechner <[email protected]>");
+MODULE_AUTHOR("Paul Cercueil <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.27.0
Hi Paul.
On Mon, Jul 27, 2020 at 06:46:08PM +0200, Paul Cercueil wrote:
> Add documentation for the Device Tree node for LCD panels based on the
> NewVision NV3052C controller.
>
> Signed-off-by: Paul Cercueil <[email protected]>
Very happy to see work on RG-350 :-)
Some feedback below.
Sam
> ---
> .../display/panel/newvision,nv3052c.yaml | 69 +++++++++++++++++++
> 1 file changed, 69 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..751a28800fc2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
> @@ -0,0 +1,69 @@
> +# 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 <[email protected]>
> +
> +description: |
> + This is a driver for 320x240 TFT panels,
The binding describes the HW, not the driver. So please re-phrase this
part.
This datasheet: https://www.phoenixdisplay.com/wp-content/uploads/2019/05/NV3052C-Datasheet-V0.2.pdf
tells that the driver supports additional resoltions.
I guess the 320x240 resolution is limited to the leadtek panel.
> + accepting a variety of input
> + streams that get adapted and scaled to the panel. The panel output has
> + 960 TFT source driver pins and 240 TFT gate driver pins, VCOM, VCOML and
> + VCOMH outputs.
> +
> + 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
> + port: true
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
Do the panel need any power?
I had expected to see a power-supply node as mandatory.
> +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>;
These would fail later due to "unevaluatedProperties: false".
Add them above like
backlight: true
power-supply: true
as done for reset-gpios for example.
> +
> + port {
> + panel_input: endpoint {
> + remote-endpoint = <&panel_output>;
> + };
> + };
> + };
> + };
Personally I prefer 4 space indent. But there is no fixed rule (yet)
what to use.
> +
> +...
> --
> 2.27.0
On Monday, 27 July 2020 21:10:52 CEST Sam Ravnborg wrote:
> > +description: |
> > + This is a driver for 320x240 TFT panels,
>
> The binding describes the HW, not the driver. So please re-phrase this
> part.
>
> This datasheet:
> https://www.phoenixdisplay.com/wp-content/uploads/2019/05/NV3052C-Dat
> asheet-V0.2.pdf tells that the driver supports additional resoltions.
> I guess the 320x240 resolution is limited to the leadtek panel.
The word "driver" is overloaded ;)
I guess "driver IC" would make it clearer.
Bye,
Maarten
Hi Paul,
Thank you for the patch.
On Mon, Jul 27, 2020 at 06:46:08PM +0200, Paul Cercueil wrote:
> Add documentation for the Device Tree node for LCD panels based on the
> NewVision NV3052C controller.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> .../display/panel/newvision,nv3052c.yaml | 69 +++++++++++++++++++
> 1 file changed, 69 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..751a28800fc2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3052c.yaml
> @@ -0,0 +1,69 @@
> +# 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
s/driver/driver IC/ (or driver chip, or controller, or any other similar
term) to avoid confusion with device drivers.
Do I understand that the NV3052C also supports control through DSI ?
Shouldn't this appear in the DT bindings ? Do I assume correctly that
the panel will be controlled either through SPI or through DSI, but not
through both ?
> +
> +maintainers:
> + - Paul Cercueil <[email protected]>
> +
> +description: |
> + This is a driver for 320x240 TFT panels, accepting a variety of input
> + streams that get adapted and scaled to the panel. The panel output has
> + 960 TFT source driver pins and 240 TFT gate driver pins, VCOM, VCOML and
> + VCOMH outputs.
> +
> + 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
According to its datasheet, that panel is 640x480 :-)
I think you need a bit of documentation to explain that two compatible
strings are needed, one matching the panel type, and a second one
matching the chip.
> +
> + - const: newvision,nv3052c
> +
> + reg:
> + maxItems: 1
> +
> + reset-gpios: true
> + port: true
The NV3052C requires multiple power supplies, I think this needs to be
taken into account here.
> +
> +required:
> + - compatible
> + - reg
> +
> +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>;
> + };
> + };
> + };
> + };
> +
> +...
--
Regards,
Laurent Pinchart