[PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Linus Walleij]

The TPO TPG110 bindings were using the DPI bindings (popular
in the fbdev subsystem) but this misses the finer points
learned in the DRM subsystem.

We need to augment the bindings for proper DRM integration:
the timings are expressed by the hardware, not put into the
device tree.

Old device trees with the DPI info will continue to work,
but no known deployments exist.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../bindings/display/panel/tpo,tpg110.txt     | 34 ++++++++-----------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
index f5e3c6f2095a..0e918076d55e 100644
--- a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
+++ b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
@@ -1,26 +1,32 @@
 TPO TPG110 Panel
 ================
 
-This binding builds on the DPI bindings, adding a few properties
-as a superset of a DPI. See panel-dpi.txt for the required DPI
-bindings.
+This panel driver can driver a variety of panels. It requires
+a few GPIO lines for control of its reset line and custom serial
+protocol.
 
 Required properties:
-- compatible : "tpo,tpg110"
+- compatible : one of:
+  "ste,nomadik-nhk15-display", "tpo,tpg110"
+  "tpo,tpg110"
 - grestb-gpios : panel reset GPIO
 - scen-gpios : serial control enable GPIO
 - scl-gpios : serial control clock line GPIO
 - sda-gpios : serial control data line GPIO
+- width-mm : see display/panel/panel-common.txt
+- height-mm : see display/panel/panel-common.txt
 
-Required nodes:
-- Video port for DPI input, see panel-dpi.txt
-- Panel timing for DPI setup, see panel-dpi.txt
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in
+media/video-interfaces.txt. This node should describe panel's video bus.
 
 Example
 -------
 
 panel {
-	compatible = "tpo,tpg110", "panel-dpi";
+	compatible = "tpo,tpg110";
+	width-mm = <116>;
+	height-mm = <87>;
 	grestb-gpios = <&stmpe_gpio44 5 GPIO_ACTIVE_LOW>;
 	scen-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
 	scl-gpios = <&gpio0 5 GPIO_ACTIVE_HIGH>;
@@ -32,16 +38,4 @@ panel {
 			remote-endpoint = <&nomadik_clcd_pads>;
 		};
 	};
-
-	panel-timing {
-		clock-frequency = <33200000>;
-		hactive = <800>;
-		hback-porch = <216>;
-		hfront-porch = <40>;
-		hsync-len = <1>;
-		vactive = <480>;
-		vback-porch = <35>;
-		vfront-porch = <10>;
-		vsync-len = <1>;
-	};
 };
-- 
2.17.1

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

[PATCH 2/2] drm/panel: Add a driver for the TPO TPG110

[Linus Walleij]

The TPO (Toppoly) TPG110 is a pretty generic display driver
similar in vein to the Ilitek 93xx devices. It is not a panel
per se but a driver used with several low-cost noname panels.

This is used on the Nomadik NHK15 combined with a OSD
OSD057VA01CT display for WVGA 800x480.

The driver is pretty minimalistic right now but can be
extended to handle non-default polarities, gamma correction
etc.

The driver is based on the baked-in code in
drivers/video/fbdev/amba-clcd-nomadik.c which will be
decomissioned once this us upstream.

The device tree bindings already exist in:
Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 MAINTAINERS                              |   7 +
 drivers/gpu/drm/panel/Kconfig            |  10 +
 drivers/gpu/drm/panel/Makefile           |   1 +
 drivers/gpu/drm/panel/panel-tpo-tpg110.c | 525 +++++++++++++++++++++++
 4 files changed, 543 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-tpo-tpg110.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d5eeff51b5f..8767a572a56f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4612,6 +4612,13 @@ DRM DRIVER FOR TDFX VIDEO CARDS
 S:	Orphan / Obsolete
 F:	drivers/gpu/drm/tdfx/
 
+DRM DRIVER FOR TPO TPG110 PANELS
+M:	Linus Walleij <linus.walleij@linaro.org>
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+S:	Maintained
+F:	drivers/gpu/drm/panel/panel-tpo-tpg110.c
+F:	Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
+
 DRM DRIVER FOR USB DISPLAYLINK VIDEO ADAPTERS
 M:	Dave Airlie <airlied@redhat.com>
 S:	Odd Fixes
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 25682ff3449a..ad946de3a72d 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -177,4 +177,14 @@ config DRM_PANEL_SITRONIX_ST7789V
 	  Say Y here if you want to enable support for the Sitronix
 	  ST7789V controller for 240x320 LCD panels
 
+config DRM_PANEL_TPO_TPG110
+	tristate "TPO TPG 800x400 panel"
+	depends on OF && GPIOLIB
+	depends on BACKLIGHT_CLASS_DEVICE
+	select VIDEOMODE_HELPERS
+	help
+	  Say Y here if you want to enable support for TPO TPG110
+	  400CH LTPS TFT LCD Single Chip Digital Driver for up to
+	  800x400 LCD panels.
+
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f26efc11d746..5136643162cc 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
+obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
diff --git a/drivers/gpu/drm/panel/panel-tpo-tpg110.c b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
new file mode 100644
index 000000000000..e2a4a5dc517a
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
@@ -0,0 +1,525 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Panel driver for the TPO TPG110 400CH LTPS TFT LCD Single Chip
+ * Digital Driver.
+ *
+ * This chip drives a TFT LCD, so it does not know what kind of
+ * display is actually connected to it, so the width and height of that
+ * display needs to be supplied from the machine configuration.
+ *
+ * Author:
+ * Linus Walleij <linus.wallei@linaro.org>
+ */
+#include <drm/drmP.h>
+#include <drm/drm_panel.h>
+
+#include <linux/bitops.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/backlight.h>
+
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#define TPG110_TEST			0x00
+#define TPG110_CHIPID			0x01
+#define TPG110_CTRL1			0x02
+#define TPG110_RES_MASK			GENMASK(2, 0)
+#define TPG110_RES_800X480		0x07
+#define TPG110_RES_640X480		0x06
+#define TPG110_RES_480X272		0x05
+#define TPG110_RES_480X640		0x04
+#define TPG110_RES_480X272_D		0x01 /* Dual scan: outputs 800x480 */
+#define TPG110_RES_400X240_D		0x00 /* Dual scan: outputs 800x480 */
+#define TPG110_CTRL2			0x03
+#define TPG110_CTRL2_PM			BIT(0)
+#define TPG110_CTRL2_RES_PM_CTRL	BIT(7)
+
+/**
+ * struct tpg110_panel_mode - lookup struct for the supported modes
+ */
+struct tpg110_panel_mode {
+	/**
+	 * @name: the name of this panel
+	 */
+	const char *name;
+	/**
+	 * @magic: the magic value from the detection register
+	 */
+	u32 magic;
+	/**
+	 * @mode: the DRM display mode for this panel
+	 */
+	struct drm_display_mode mode;
+	/**
+	 * @bus_flags: the DRM bus flags for this panel e.g. inverted clock
+	 */
+	u32 bus_flags;
+};
+
+/**
+ * struct tpg110 - state container for the TPG110 panel
+ */
+struct tpg110 {
+	/**
+	 * @dev: the container device
+	 */
+	struct device *dev;
+	/**
+	 * @panel: the DRM panel instance for this device
+	 */
+	struct drm_panel panel;
+	/**
+	 * @backlight: backlight for this panel
+	 */
+	struct backlight_device *backlight;
+	/**
+	 * @panel_type: the panel mode as detected
+	 */
+	const struct tpg110_panel_mode *panel_mode;
+	/**
+	 * @width: the width of this panel in mm
+	 */
+	u32 width;
+	/**
+	 * @height: the height of this panel in mm
+	 */
+	u32 height;
+	/**
+	 * @grestb: reset GPIO line
+	 */
+	struct gpio_desc *grestb;
+	/**
+	 * @scen: scen GPIO line
+	 */
+	struct gpio_desc *scen;
+	/**
+	 * @scl: scl (clock) GPIO line
+	 */
+	struct gpio_desc *scl;
+	/**
+	 * @sda: sda (data) GPIO line
+	 */
+	struct gpio_desc *sda;
+};
+
+/*
+ * TPG110 modes, these are the simple modes, the dualscan modes that
+ * take 400x240 or 480x272 in and display as 800x480 are not listed.
+ */
+static const struct tpg110_panel_mode tpg110_modes[] = {
+	{
+		.name = "800x480 RGB",
+		.magic = TPG110_RES_800X480,
+		.mode = {
+			.clock = 33200,
+			.hdisplay = 800,
+			.hsync_start = 800 + 40,
+			.hsync_end = 800 + 40 + 1,
+			.htotal = 800 + 40 + 1 + 216,
+			.vdisplay = 480,
+			.vsync_start = 480 + 10,
+			.vsync_end = 480 + 10 + 1,
+			.vtotal = 480 + 10 + 1 + 35,
+			.vrefresh = 60,
+		},
+	},
+	{
+		.name = "640x480 RGB",
+		.magic = TPG110_RES_640X480,
+		.mode = {
+			.clock = 25200,
+			.hdisplay = 640,
+			.hsync_start = 640 + 24,
+			.hsync_end = 640 + 24 + 1,
+			.htotal = 640 + 24 + 1 + 136,
+			.vdisplay = 480,
+			.vsync_start = 480 + 18,
+			.vsync_end = 480 + 18 + 1,
+			.vtotal = 480 + 18 + 1 + 27,
+			.vrefresh = 60,
+		},
+	},
+	{
+		.name = "480x272 RGB",
+		.magic = TPG110_RES_480X272,
+		.mode = {
+			.clock = 9000,
+			.hdisplay = 480,
+			.hsync_start = 480 + 2,
+			.hsync_end = 480 + 2 + 1,
+			.htotal = 480 + 2 + 1 + 43,
+			.vdisplay = 272,
+			.vsync_start = 272 + 2,
+			.vsync_end = 272 + 2 + 1,
+			.vtotal = 272 + 2 + 1 + 12,
+			.vrefresh = 60,
+		},
+	},
+	{
+		.name = "480x640 RGB",
+		.magic = TPG110_RES_480X640,
+		.mode = {
+			.clock = 20500,
+			.hdisplay = 480,
+			.hsync_start = 480 + 2,
+			.hsync_end = 480 + 2 + 1,
+			.htotal = 480 + 2 + 1 + 43,
+			.vdisplay = 640,
+			.vsync_start = 640 + 4,
+			.vsync_end = 640 + 4 + 1,
+			.vtotal = 640 + 4 + 1 + 8,
+			.vrefresh = 60,
+		},
+	},
+	{
+		.name = "400x240 RGB",
+		.magic = TPG110_RES_400X240_D,
+		.mode = {
+			.clock = 8300,
+			.hdisplay = 400,
+			.hsync_start = 400 + 20,
+			.hsync_end = 400 + 20 + 1,
+			.htotal = 400 + 20 + 1 + 108,
+			.vdisplay = 240,
+			.vsync_start = 240 + 2,
+			.vsync_end = 240 + 2 + 1,
+			.vtotal = 240 + 2 + 1 + 20,
+			.vrefresh = 60,
+		},
+	},
+};
+
+static inline struct tpg110 *
+to_tpg110(struct drm_panel *panel)
+{
+	return container_of(panel, struct tpg110, panel);
+}
+
+static u8 tpg110_readwrite_reg(struct tpg110 *tpg, bool write,
+			       u8 address, u8 outval)
+{
+	int i;
+	u8 inval = 0;
+
+	/* Assert SCEN */
+	gpiod_set_value_cansleep(tpg->scen, 1);
+	ndelay(150);
+	/* Hammer out the address */
+	for (i = 5; i >= 0; i--) {
+		if (address & BIT(i))
+			gpiod_set_value_cansleep(tpg->sda, 1);
+		else
+			gpiod_set_value_cansleep(tpg->sda, 0);
+		ndelay(150);
+		/* Send an SCL pulse */
+		gpiod_set_value_cansleep(tpg->scl, 1);
+		ndelay(160);
+		gpiod_set_value_cansleep(tpg->scl, 0);
+		ndelay(160);
+	}
+
+	if (write) {
+		/* WRITE */
+		gpiod_set_value_cansleep(tpg->sda, 0);
+	} else {
+		/* READ */
+		gpiod_set_value_cansleep(tpg->sda, 1);
+	}
+	ndelay(150);
+	/* Send an SCL pulse */
+	gpiod_set_value_cansleep(tpg->scl, 1);
+	ndelay(160);
+	gpiod_set_value_cansleep(tpg->scl, 0);
+	ndelay(160);
+
+	if (!write)
+		/* HiZ turn-around cycle */
+		gpiod_direction_input(tpg->sda);
+	ndelay(150);
+	/* Send an SCL pulse */
+	gpiod_set_value_cansleep(tpg->scl, 1);
+	ndelay(160);
+	gpiod_set_value_cansleep(tpg->scl, 0);
+	ndelay(160);
+
+	/* Hammer in/out the data */
+	for (i = 7; i >= 0; i--) {
+		int value;
+
+		if (write) {
+			value = !!(outval & BIT(i));
+			gpiod_set_value_cansleep(tpg->sda, value);
+		} else {
+			value = gpiod_get_value(tpg->sda);
+			if (value)
+				inval |= BIT(i);
+		}
+		ndelay(150);
+		/* Send an SCL pulse */
+		gpiod_set_value_cansleep(tpg->scl, 1);
+		ndelay(160);
+		gpiod_set_value_cansleep(tpg->scl, 0);
+		ndelay(160);
+	}
+
+	gpiod_direction_output(tpg->sda, 0);
+	/* Deassert SCEN */
+	gpiod_set_value_cansleep(tpg->scen, 0);
+	/* Satisfies SCEN pulse width */
+	udelay(1);
+
+	return inval;
+}
+
+static u8 tpg110_read_reg(struct tpg110 *tpg, u8 address)
+{
+	return tpg110_readwrite_reg(tpg, false, address, 0);
+}
+
+static void tpg110_write_reg(struct tpg110 *tpg, u8 address, u8 outval)
+{
+	tpg110_readwrite_reg(tpg, true, address, outval);
+}
+
+static int tpg110_startup(struct tpg110 *tpg)
+{
+	u8 val;
+	int i;
+
+	/* De-assert the reset signal */
+	gpiod_set_value_cansleep(tpg->grestb, 0);
+	mdelay(1);
+	dev_info(tpg->dev, "de-asserted GRESTB\n");
+
+	/* Test display communication */
+	tpg110_write_reg(tpg, TPG110_TEST, 0x55);
+	val = tpg110_read_reg(tpg, TPG110_TEST);
+	if (val != 0x55) {
+		dev_err(tpg->dev, "failed communication test\n");
+		return -ENODEV;
+	}
+
+	val = tpg110_read_reg(tpg, TPG110_CHIPID);
+	dev_info(tpg->dev, "TPG110 chip ID: %d version: %d\n",
+		 val >> 4, val & 0x0f);
+
+	/* Show display resolution */
+	val = tpg110_read_reg(tpg, TPG110_CTRL1);
+	val &= TPG110_RES_MASK;
+	switch (val) {
+	case TPG110_RES_400X240_D:
+		dev_info(tpg->dev,
+			 "IN 400x240 RGB -> OUT 800x480 RGB (dual scan)");
+		break;
+	case TPG110_RES_480X272_D:
+		dev_info(tpg->dev,
+			 "IN 480x272 RGB -> OUT 800x480 RGB (dual scan)");
+		break;
+	case TPG110_RES_480X640:
+		dev_info(tpg->dev, "480x640 RGB");
+		break;
+	case TPG110_RES_480X272:
+		dev_info(tpg->dev, "480x272 RGB");
+		break;
+	case TPG110_RES_640X480:
+		dev_info(tpg->dev, "640x480 RGB");
+		break;
+	case TPG110_RES_800X480:
+		dev_info(tpg->dev, "800x480 RGB");
+		break;
+	default:
+		dev_info(tpg->dev, "ILLEGAL RESOLUTION");
+		break;
+	}
+
+	/* From the producer side, this is the same resolution */
+	if (val == TPG110_RES_480X272_D)
+		val = TPG110_RES_480X272;
+
+	for (i = 0; i < ARRAY_SIZE(tpg110_modes); i++) {
+		const struct tpg110_panel_mode *pm;
+
+		pm = &tpg110_modes[i];
+		if (pm->magic == val) {
+			tpg->panel_mode = pm;
+			break;
+		}
+	}
+	if (i == ARRAY_SIZE(tpg110_modes)) {
+		dev_err(tpg->dev, "unsupported mode (%02x) detected\n",
+			val);
+		return -ENODEV;
+	}
+
+	val = tpg110_read_reg(tpg, TPG110_CTRL2);
+	dev_info(tpg->dev, "resolution and standby is controlled by %s\n",
+		 (val & TPG110_CTRL2_RES_PM_CTRL) ? "software" : "hardware");
+	/* Take control over resolution and standby */
+	val |= TPG110_CTRL2_RES_PM_CTRL;
+	tpg110_write_reg(tpg, TPG110_CTRL2, val);
+
+	return 0;
+}
+
+static int tpg110_disable(struct drm_panel *panel)
+{
+	struct tpg110 *tpg = to_tpg110(panel);
+	u8 val;
+
+	/* Put chip into standby */
+	val = tpg110_read_reg(tpg, TPG110_CTRL2_PM);
+	val &= ~TPG110_CTRL2_PM;
+	tpg110_write_reg(tpg, TPG110_CTRL2_PM, val);
+
+	if (tpg->backlight) {
+		tpg->backlight->props.power = FB_BLANK_POWERDOWN;
+		tpg->backlight->props.state |= BL_CORE_FBBLANK;
+		backlight_update_status(tpg->backlight);
+	}
+
+	return 0;
+}
+
+static int tpg110_enable(struct drm_panel *panel)
+{
+	struct tpg110 *tpg = to_tpg110(panel);
+	u8 val;
+
+	if (tpg->backlight) {
+		tpg->backlight->props.state &= ~BL_CORE_FBBLANK;
+		tpg->backlight->props.power = FB_BLANK_UNBLANK;
+		backlight_update_status(tpg->backlight);
+	}
+
+	/* Take chip out of standby */
+	val = tpg110_read_reg(tpg, TPG110_CTRL2_PM);
+	val |= TPG110_CTRL2_PM;
+	tpg110_write_reg(tpg, TPG110_CTRL2_PM, val);
+
+	return 0;
+}
+
+/**
+ * tpg110_get_modes() - return the appropriate mode
+ * @panel: the panel to get the mode for
+ *
+ * This currently does not present a forest of modes, instead it
+ * presents the mode that is configured for the system under use,
+ * and which is detected by reading the registers of the display.
+ */
+static int tpg110_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct tpg110 *tpg = to_tpg110(panel);
+	struct drm_display_mode *mode;
+
+	strncpy(connector->display_info.name, tpg->panel_mode->name,
+		DRM_DISPLAY_INFO_LEN);
+	connector->display_info.width_mm = tpg->width;
+	connector->display_info.height_mm = tpg->height;
+	connector->display_info.bus_flags = tpg->panel_mode->bus_flags;
+
+	mode = drm_mode_duplicate(panel->drm, &tpg->panel_mode->mode);
+	drm_mode_set_name(mode);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+
+	mode->width_mm = tpg->width;
+	mode->height_mm = tpg->height;
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs tpg110_drm_funcs = {
+	.disable = tpg110_disable,
+	.enable = tpg110_enable,
+	.get_modes = tpg110_get_modes,
+};
+
+static int tpg110_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *backlight;
+	struct tpg110 *tpg;
+	int ret;
+
+	tpg = devm_kzalloc(dev, sizeof(*tpg), GFP_KERNEL);
+	if (!tpg)
+		return -ENOMEM;
+	tpg->dev = dev;
+
+	/* We get the physical display dimensions from the DT */
+	ret = of_property_read_u32(np, "width-mm", &tpg->width);
+	if (ret)
+		dev_err(dev, "no panel width specified\n");
+	ret = of_property_read_u32(np, "height-mm", &tpg->height);
+	if (ret)
+		dev_err(dev, "no panel height specified\n");
+
+	/* Look for some optional backlight */
+	backlight = of_parse_phandle(np, "backlight", 0);
+	if (backlight) {
+		tpg->backlight = of_find_backlight_by_node(backlight);
+		of_node_put(backlight);
+
+		if (!tpg->backlight)
+			return -EPROBE_DEFER;
+	}
+
+	/* This asserts the GRESTB signal, putting the display into reset */
+	tpg->grestb = devm_gpiod_get(dev, "grestb", GPIOD_OUT_HIGH);
+	if (IS_ERR(tpg->grestb)) {
+		dev_err(dev, "no GRESTB GPIO\n");
+		return -ENODEV;
+	}
+	tpg->scen = devm_gpiod_get(dev, "scen", GPIOD_OUT_LOW);
+	if (IS_ERR(tpg->scen)) {
+		dev_err(dev, "no SCEN GPIO\n");
+		return -ENODEV;
+	}
+	tpg->scl = devm_gpiod_get(dev, "scl", GPIOD_OUT_LOW);
+	if (IS_ERR(tpg->scl)) {
+		dev_err(dev, "no SCL GPIO\n");
+		return -ENODEV;
+	}
+	tpg->sda = devm_gpiod_get(dev, "sda", GPIOD_OUT_LOW);
+	if (IS_ERR(tpg->sda)) {
+		dev_err(dev, "no SDA GPIO\n");
+		return -ENODEV;
+	}
+
+	ret = tpg110_startup(tpg);
+	if (ret)
+		return ret;
+
+	drm_panel_init(&tpg->panel);
+	tpg->panel.dev = dev;
+	tpg->panel.funcs = &tpg110_drm_funcs;
+
+	return drm_panel_add(&tpg->panel);
+}
+
+static const struct of_device_id tpg110_match[] = {
+	{ .compatible = "tpo,tpg110", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, tpg110_match);
+
+static struct platform_driver tpg110_driver = {
+	.probe		= tpg110_probe,
+	.driver		= {
+		.name	= "tpo-tpg110-panel",
+		.of_match_table = tpg110_match,
+	},
+};
+module_platform_driver(tpg110_driver);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("TPO TPG110 panel driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

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

Re: [PATCH 2/2] drm/panel: Add a driver for the TPO TPG110

[kbuild test robot]

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

Hi Linus,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tegra-drm/drm/tegra/for-next]
[also build test WARNING on v4.18-rc1 next-20180621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Linus-Walleij/drm-panel-Augment-the-TPO-TPG110-bindings/20180622-025655
base:   git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/panel/panel-tpo-tpg110.c: In function 'tpg110_get_modes':
>> drivers/gpu/drm/panel/panel-tpo-tpg110.c:420:2: warning: 'strncpy' specified bound 32 equals destination size [-Wstringop-truncation]
     strncpy(connector->display_info.name, tpg->panel_mode->name,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      DRM_DISPLAY_INFO_LEN);
      ~~~~~~~~~~~~~~~~~~~~~

vim +/strncpy +420 drivers/gpu/drm/panel/panel-tpo-tpg110.c

   405	
   406	/**
   407	 * tpg110_get_modes() - return the appropriate mode
   408	 * @panel: the panel to get the mode for
   409	 *
   410	 * This currently does not present a forest of modes, instead it
   411	 * presents the mode that is configured for the system under use,
   412	 * and which is detected by reading the registers of the display.
   413	 */
   414	static int tpg110_get_modes(struct drm_panel *panel)
   415	{
   416		struct drm_connector *connector = panel->connector;
   417		struct tpg110 *tpg = to_tpg110(panel);
   418		struct drm_display_mode *mode;
   419	
 > 420		strncpy(connector->display_info.name, tpg->panel_mode->name,
   421			DRM_DISPLAY_INFO_LEN);
   422		connector->display_info.width_mm = tpg->width;
   423		connector->display_info.height_mm = tpg->height;
   424		connector->display_info.bus_flags = tpg->panel_mode->bus_flags;
   425	
   426		mode = drm_mode_duplicate(panel->drm, &tpg->panel_mode->mode);
   427		drm_mode_set_name(mode);
   428		mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
   429	
   430		mode->width_mm = tpg->width;
   431		mode->height_mm = tpg->height;
   432	
   433		drm_mode_probed_add(connector, mode);
   434	
   435		return 1;
   436	}
   437	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49880 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Rob Herring]

On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:
> The TPO TPG110 bindings were using the DPI bindings (popular
> in the fbdev subsystem) but this misses the finer points
> learned in the DRM subsystem.
> 
> We need to augment the bindings for proper DRM integration:
> the timings are expressed by the hardware, not put into the
> device tree.
> 
> Old device trees with the DPI info will continue to work,
> but no known deployments exist.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/display/panel/tpo,tpg110.txt     | 34 ++++++++-----------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
> index f5e3c6f2095a..0e918076d55e 100644
> --- a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
> +++ b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
> @@ -1,26 +1,32 @@
>  TPO TPG110 Panel
>  ================
>  
> -This binding builds on the DPI bindings, adding a few properties
> -as a superset of a DPI. See panel-dpi.txt for the required DPI
> -bindings.
> +This panel driver can driver a variety of panels. It requires

s/can driver/can drive/

Though what a driver supports is irrelevant to the binding...

If you remove timings, how does it drive a variety of panels? Just by 
compatible? That would mean "tpo,tpg110" alone is not valid nor useful 
as a fallback.

> +a few GPIO lines for control of its reset line and custom serial
> +protocol.
>  
>  Required properties:
> -- compatible : "tpo,tpg110"
> +- compatible : one of:
> +  "ste,nomadik-nhk15-display", "tpo,tpg110"
> +  "tpo,tpg110"
>  - grestb-gpios : panel reset GPIO
>  - scen-gpios : serial control enable GPIO
>  - scl-gpios : serial control clock line GPIO
>  - sda-gpios : serial control data line GPIO

I2C? That should be done differently...

> +- width-mm : see display/panel/panel-common.txt
> +- height-mm : see display/panel/panel-common.txt
>  
> -Required nodes:
> -- Video port for DPI input, see panel-dpi.txt
> -- Panel timing for DPI setup, see panel-dpi.txt
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in
> +media/video-interfaces.txt. This node should describe panel's video bus.
>  
>  Example
>  -------
>  
>  panel {
> -	compatible = "tpo,tpg110", "panel-dpi";
> +	compatible = "tpo,tpg110";
> +	width-mm = <116>;
> +	height-mm = <87>;
>  	grestb-gpios = <&stmpe_gpio44 5 GPIO_ACTIVE_LOW>;
>  	scen-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
>  	scl-gpios = <&gpio0 5 GPIO_ACTIVE_HIGH>;
> @@ -32,16 +38,4 @@ panel {
>  			remote-endpoint = <&nomadik_clcd_pads>;
>  		};
>  	};
> -
> -	panel-timing {
> -		clock-frequency = <33200000>;
> -		hactive = <800>;
> -		hback-porch = <216>;
> -		hfront-porch = <40>;
> -		hsync-len = <1>;
> -		vactive = <480>;
> -		vback-porch = <35>;
> -		vfront-porch = <10>;
> -		vsync-len = <1>;
> -	};
>  };
> -- 
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Linus Walleij]

On Wed, Jun 27, 2018 at 7:21 PM Rob Herring <robh@kernel.org> wrote:
> On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:

> > +This panel driver can driver a variety of panels. It requires
>
> s/can driver/can drive/
>
> Though what a driver supports is irrelevant to the binding...

It it not a software driver the text is referring to. It is a
electrical interface to a panel. Like how a TTL circuit connected
to a LED is referred to as a "LED driver", it's simply what the
industry calls these things.

So there are two things: the panel driver and the panel, the
same panel driver is used with several panels. What the
electronics engineer will do is put a panel driver like this
into his design and then connect some panel s/he finds
in the right quantity in the streets of Shenzhen.

> If you remove timings, how does it drive a variety of panels? Just by
> compatible?

Yes.

Like we did for
Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt
which is similar to this.

In fact I think many panel drivers are just sloppily slipping in
under the radar as "panels" in our bindings.

>That would mean "tpo,tpg110" alone is not valid nor useful
> as a fallback.

Actually it is. The hardware is wired up to the desired
resolution with hardware straps, which appear in
the registers the (software) driver can read out so
this is ideally self-describing hardware.

But for the event that something needs tweaking in the
future, like we overspecify say SoCs, I include the
exact system on which it is deployd as a separate
compatible string.

> > +a few GPIO lines for control of its reset line and custom serial
> > +protocol.
> >
> >  Required properties:
> > -- compatible : "tpo,tpg110"
> > +- compatible : one of:
> > +  "ste,nomadik-nhk15-display", "tpo,tpg110"
> > +  "tpo,tpg110"
> >  - grestb-gpios : panel reset GPIO
> >  - scen-gpios : serial control enable GPIO
> >  - scl-gpios : serial control clock line GPIO
> >  - sda-gpios : serial control data line GPIO
>
> I2C? That should be done differently...

It is not I2C, the lines are just named confusingly
similar. None of the I2C (-like) protocols apply.
I was similarly confused when I first implemented it.

(Maybe I should add a comment to explain this.)

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

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Andrzej Hajda]

On 01.07.2018 21:01, Linus Walleij wrote:
> On Wed, Jun 27, 2018 at 7:21 PM Rob Herring <robh@kernel.org> wrote:
>> On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:
>>> +This panel driver can driver a variety of panels. It requires
>> s/can driver/can drive/
>>
>> Though what a driver supports is irrelevant to the binding...
> It it not a software driver the text is referring to. It is a
> electrical interface to a panel. Like how a TTL circuit connected
> to a LED is referred to as a "LED driver", it's simply what the
> industry calls these things.
>
> So there are two things: the panel driver and the panel, the
> same panel driver is used with several panels. What the
> electronics engineer will do is put a panel driver like this
> into his design and then connect some panel s/he finds
> in the right quantity in the streets of Shenzhen.
>
>> If you remove timings, how does it drive a variety of panels? Just by
>> compatible?
> Yes.
>
> Like we did for
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt
> which is similar to this.
>
> In fact I think many panel drivers are just sloppily slipping in
> under the radar as "panels" in our bindings.
>
>> That would mean "tpo,tpg110" alone is not valid nor useful
>> as a fallback.
> Actually it is. The hardware is wired up to the desired
> resolution with hardware straps, which appear in
> the registers the (software) driver can read out so
> this is ideally self-describing hardware.
>
> But for the event that something needs tweaking in the
> future, like we overspecify say SoCs, I include the
> exact system on which it is deployd as a separate
> compatible string.
>
>>> +a few GPIO lines for control of its reset line and custom serial
>>> +protocol.
>>>
>>>  Required properties:
>>> -- compatible : "tpo,tpg110"
>>> +- compatible : one of:
>>> +  "ste,nomadik-nhk15-display", "tpo,tpg110"
>>> +  "tpo,tpg110"
>>>  - grestb-gpios : panel reset GPIO
>>>  - scen-gpios : serial control enable GPIO
>>>  - scl-gpios : serial control clock line GPIO
>>>  - sda-gpios : serial control data line GPIO
>> I2C? That should be done differently...
> It is not I2C, the lines are just named confusingly
> similar. None of the I2C (-like) protocols apply.
> I was similarly confused when I first implemented it.
>
> (Maybe I should add a comment to explain this.)

I have grepped Internet, out of curiosity, and it seems it is so-called
3-wire spi [1].

[1]:
http://aitendo3.sakura.ne.jp/aitendo_data/product_img/lcd/tft/T43P00/TPG110%20Customer%20Spec_0.6.pdf


Regards
Andrzej

>
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Rob Herring]

On Sun, Jul 1, 2018 at 1:01 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Jun 27, 2018 at 7:21 PM Rob Herring <robh@kernel.org> wrote:
> > On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:
>
> > > +This panel driver can driver a variety of panels. It requires
> >
> > s/can driver/can drive/
> >
> > Though what a driver supports is irrelevant to the binding...
>
> It it not a software driver the text is referring to. It is a
> electrical interface to a panel. Like how a TTL circuit connected
> to a LED is referred to as a "LED driver", it's simply what the
> industry calls these things.

Yes, I'm aware of the term, but I see *far* more of the other use of "driver".

Can you say "panel driver IC" to clarify.

> So there are two things: the panel driver and the panel, the
> same panel driver is used with several panels. What the
> electronics engineer will do is put a panel driver like this
> into his design and then connect some panel s/he finds
> in the right quantity in the streets of Shenzhen.

Yep. I've had the fun of trying to figure out how to get panels
working and having the piecemeal documentation of the driver IC
without all the hook-up details...

> > If you remove timings, how does it drive a variety of panels? Just by
> > compatible?
>
> Yes.
>
> Like we did for
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt
> which is similar to this.

According to that, there's only 1 panel supported.

> In fact I think many panel drivers are just sloppily slipping in
> under the radar as "panels" in our bindings.

Indeed. I'm not sure that trying to split driver ICs and panels is
really feasible at least up front given how poor the documentation is
typically.

> >That would mean "tpo,tpg110" alone is not valid nor useful
> > as a fallback.
>
> Actually it is. The hardware is wired up to the desired
> resolution with hardware straps, which appear in
> the registers the (software) driver can read out so
> this is ideally self-describing hardware.

Nice. Can you add that detail.

> But for the event that something needs tweaking in the
> future, like we overspecify say SoCs, I include the
> exact system on which it is deployd as a separate
> compatible string.
>
> > > +a few GPIO lines for control of its reset line and custom serial
> > > +protocol.
> > >
> > >  Required properties:
> > > -- compatible : "tpo,tpg110"
> > > +- compatible : one of:
> > > +  "ste,nomadik-nhk15-display", "tpo,tpg110"
> > > +  "tpo,tpg110"
> > >  - grestb-gpios : panel reset GPIO

Also, use 'reset-gpios' as that is the standard name.

> > >  - scen-gpios : serial control enable GPIO
> > >  - scl-gpios : serial control clock line GPIO
> > >  - sda-gpios : serial control data line GPIO
> >
> > I2C? That should be done differently...
>
> It is not I2C, the lines are just named confusingly
> similar. None of the I2C (-like) protocols apply.
> I was similarly confused when I first implemented it.

If this is 3-wire SPI then as Andrzej says, then still it should be
done as spi-gpio. Which really just means these gpio properties go
away and this should be a SPI child node.

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