LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4 0/3] drm/panel: Add Ilitek ILI9881c controller driver
@ 2018-05-04 15:23 Maxime Ripard
  2018-05-04 15:23 ` [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Maxime Ripard @ 2018-05-04 15:23 UTC (permalink / raw)
  To: Thierry Reding, Chen-Yu Tsai, Maxime Ripard
  Cc: dri-devel, Gustavo Padovan, Daniel Vetter, Maarten Lankhorst,
	Sean Paul, linux-kernel

Hi,

Here is the next version of the patches to add the support for the Ilitek
ILI9881c panel controller.

This used to be a part of the larger DSI support series for the Allwinner
SoCs whose patches have been since merged and are obviously not part of
this series anymore.

Let me know what you think,
Maxime

Changes from v4:
  - Updated the copyright notice
  - Made the init structure const

Changes from v3:
  - Rebased on top of current drm-misc-next
  - Switched to SPDX license header
  - Made the ECC array const
  - Split the big DSI patch into two, one to add the DSI driver and one to
    add the TCON bits.
  - Removed the dithering code
  - Changed the DT labels to remove the indices
  - Used sleeps instead of delays in the panel driver
  - Used the backlight_enable / _disable functions
  - Added Chen-Yu's Reviewed-by

Changes from v2:
  - Added a ports node under the DSI node
  - Changed the huarui panel driver to an ili9881c driver
  - Changed the panel vendor to bananapi
  - Made the init table static in the panel driver
  - Dropped the huarui vendor patch for the DT doc.

Changes from v1:
  - Rebased on 4.16-rc1
  - Constified a few function arguments and structures
  - Reworked the DT binding example a bit
  - Reworked the panel driver to check for DSI return codes, and use DCS
    helpers when possible

Maxime Ripard (3):
  dt-bindings: panel: Add the Ilitek ILI9881c panel documentation
  drm/panel: Add Ilitek ILI9881c panel driver
  [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display

 Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt |  20 +++-
 arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts                        |  39 ++++++-
 drivers/gpu/drm/panel/Kconfig                                       |   9 +-
 drivers/gpu/drm/panel/Makefile                                      |   1 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c                       | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 557 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c

base-commit: a08fc7c8056e75b08285a2ad955228002dcd86bc
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation
  2018-05-04 15:23 [PATCH v4 0/3] drm/panel: Add Ilitek ILI9881c controller driver Maxime Ripard
@ 2018-05-04 15:23 ` Maxime Ripard
  2018-05-11 15:40   ` Linus Walleij
  2018-05-11 15:55   ` Linus Walleij
  2018-05-04 15:23 ` [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
  2018-05-04 15:23 ` [PATCH v4 3/3] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard
  2 siblings, 2 replies; 11+ messages in thread
From: Maxime Ripard @ 2018-05-04 15:23 UTC (permalink / raw)
  To: Thierry Reding, Chen-Yu Tsai, Maxime Ripard
  Cc: dri-devel, Gustavo Padovan, Daniel Vetter, Maarten Lankhorst,
	Sean Paul, linux-kernel

The LHR050H41 from BananaPi is a 1280x700 4-lanes DSI panel based on the
ILI9881c from Ilitek.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt

diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt
new file mode 100644
index 000000000000..df05e8bb4851
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.txt
@@ -0,0 +1,20 @@
+Ilitek ILI9881c based MIPI-DSI panels
+
+Required properties:
+  - compatible: must be "ilitek,ili9881c" and one of:
+    * "bananapi,lhr050h41"
+  - reg: DSI virtual channel used by that screen
+  - power-gpios: a GPIO phandle for the power pin
+  - reset-gpios: a GPIO phandle for the reset pin
+
+Optional properties:
+  - backlight: phandle to the backlight used
+
+Example:
+panel@0 {
+	compatible = "bananapi,lhr050h41", "ilitek,ili9881c";
+	reg = <0>;
+	power-gpios = <&pio 1 7 GPIO_ACTIVE_HIGH>; /* PB07 */
+	reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL05 */
+	backlight = <&pwm_bl>;
+};
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver
  2018-05-04 15:23 [PATCH v4 0/3] drm/panel: Add Ilitek ILI9881c controller driver Maxime Ripard
  2018-05-04 15:23 ` [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
@ 2018-05-04 15:23 ` Maxime Ripard
  2018-05-05  3:48   ` kbuild test robot
                     ` (2 more replies)
  2018-05-04 15:23 ` [PATCH v4 3/3] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard
  2 siblings, 3 replies; 11+ messages in thread
From: Maxime Ripard @ 2018-05-04 15:23 UTC (permalink / raw)
  To: Thierry Reding, Chen-Yu Tsai, Maxime Ripard
  Cc: dri-devel, Gustavo Padovan, Daniel Vetter, Maarten Lankhorst,
	Sean Paul, linux-kernel

The LHR050H41 panel is the panel shipped with the BananaPi M2-Magic, and is
based on the Ilitek ILI9881c Controller. Add a driver for it, modelled
after the other Ilitek controller drivers.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/panel/Kconfig                 |   9 +-
 drivers/gpu/drm/panel/Makefile                |   1 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 488 +++++++++++++++++++-
 3 files changed, 498 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 25682ff3449a..6020c30a33b3 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -46,6 +46,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_ILI9881C
+	tristate "Ilitek ILI9881C-based panels"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y if you want to enable support for panels based on the
+	  Ilitek ILI9881c controller.
+
 config DRM_PANEL_INNOLUX_P079ZCA
 	tristate "Innolux P079ZCA panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f26efc11d746..5ccaaa9d13af 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
 obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.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
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
new file mode 100644
index 000000000000..7c7e308cdb22
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -0,0 +1,488 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017-2018, Bootlin
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <linux/gpio/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+struct ili9881c {
+	struct drm_panel	panel;
+	struct mipi_dsi_device	*dsi;
+
+	struct backlight_device *backlight;
+	struct gpio_desc	*power;
+	struct gpio_desc	*reset;
+};
+
+enum ili9881c_op {
+	ILI9881C_SWITCH_PAGE,
+	ILI9881C_COMMAND,
+};
+
+struct ili9881c_instr {
+	enum ili9881c_op	op;
+
+	union arg {
+		struct cmd {
+			u8	cmd;
+			u8	data;
+		} cmd;
+		u8	page;
+	} arg;
+};
+
+#define ILI9881C_SWITCH_PAGE_INSTR(_page)	\
+	{					\
+		.op = ILI9881C_SWITCH_PAGE,	\
+		.arg = {			\
+			.page = (_page),	\
+		},				\
+	}
+
+#define ILI9881C_COMMAND_INSTR(_cmd, _data)		\
+	{						\
+		.op = ILI9881C_COMMAND,		\
+		.arg = {				\
+			.cmd = {			\
+				.cmd = (_cmd),		\
+				.data = (_data),	\
+			},				\
+		},					\
+	}
+
+static const struct ili9881c_instr ili9881c_init[] = {
+	ILI9881C_SWITCH_PAGE_INSTR(3),
+	ILI9881C_COMMAND_INSTR(0x01, 0x00),
+	ILI9881C_COMMAND_INSTR(0x02, 0x00),
+	ILI9881C_COMMAND_INSTR(0x03, 0x73),
+	ILI9881C_COMMAND_INSTR(0x04, 0x03),
+	ILI9881C_COMMAND_INSTR(0x05, 0x00),
+	ILI9881C_COMMAND_INSTR(0x06, 0x06),
+	ILI9881C_COMMAND_INSTR(0x07, 0x06),
+	ILI9881C_COMMAND_INSTR(0x08, 0x00),
+	ILI9881C_COMMAND_INSTR(0x09, 0x18),
+	ILI9881C_COMMAND_INSTR(0x0a, 0x04),
+	ILI9881C_COMMAND_INSTR(0x0b, 0x00),
+	ILI9881C_COMMAND_INSTR(0x0c, 0x02),
+	ILI9881C_COMMAND_INSTR(0x0d, 0x03),
+	ILI9881C_COMMAND_INSTR(0x0e, 0x00),
+	ILI9881C_COMMAND_INSTR(0x0f, 0x25),
+	ILI9881C_COMMAND_INSTR(0x10, 0x25),
+	ILI9881C_COMMAND_INSTR(0x11, 0x00),
+	ILI9881C_COMMAND_INSTR(0x12, 0x00),
+	ILI9881C_COMMAND_INSTR(0x13, 0x00),
+	ILI9881C_COMMAND_INSTR(0x14, 0x00),
+	ILI9881C_COMMAND_INSTR(0x15, 0x00),
+	ILI9881C_COMMAND_INSTR(0x16, 0x0C),
+	ILI9881C_COMMAND_INSTR(0x17, 0x00),
+	ILI9881C_COMMAND_INSTR(0x18, 0x00),
+	ILI9881C_COMMAND_INSTR(0x19, 0x00),
+	ILI9881C_COMMAND_INSTR(0x1a, 0x00),
+	ILI9881C_COMMAND_INSTR(0x1b, 0x00),
+	ILI9881C_COMMAND_INSTR(0x1c, 0x00),
+	ILI9881C_COMMAND_INSTR(0x1d, 0x00),
+	ILI9881C_COMMAND_INSTR(0x1e, 0xC0),
+	ILI9881C_COMMAND_INSTR(0x1f, 0x80),
+	ILI9881C_COMMAND_INSTR(0x20, 0x04),
+	ILI9881C_COMMAND_INSTR(0x21, 0x01),
+	ILI9881C_COMMAND_INSTR(0x22, 0x00),
+	ILI9881C_COMMAND_INSTR(0x23, 0x00),
+	ILI9881C_COMMAND_INSTR(0x24, 0x00),
+	ILI9881C_COMMAND_INSTR(0x25, 0x00),
+	ILI9881C_COMMAND_INSTR(0x26, 0x00),
+	ILI9881C_COMMAND_INSTR(0x27, 0x00),
+	ILI9881C_COMMAND_INSTR(0x28, 0x33),
+	ILI9881C_COMMAND_INSTR(0x29, 0x03),
+	ILI9881C_COMMAND_INSTR(0x2a, 0x00),
+	ILI9881C_COMMAND_INSTR(0x2b, 0x00),
+	ILI9881C_COMMAND_INSTR(0x2c, 0x00),
+	ILI9881C_COMMAND_INSTR(0x2d, 0x00),
+	ILI9881C_COMMAND_INSTR(0x2e, 0x00),
+	ILI9881C_COMMAND_INSTR(0x2f, 0x00),
+	ILI9881C_COMMAND_INSTR(0x30, 0x00),
+	ILI9881C_COMMAND_INSTR(0x31, 0x00),
+	ILI9881C_COMMAND_INSTR(0x32, 0x00),
+	ILI9881C_COMMAND_INSTR(0x33, 0x00),
+	ILI9881C_COMMAND_INSTR(0x34, 0x04),
+	ILI9881C_COMMAND_INSTR(0x35, 0x00),
+	ILI9881C_COMMAND_INSTR(0x36, 0x00),
+	ILI9881C_COMMAND_INSTR(0x37, 0x00),
+	ILI9881C_COMMAND_INSTR(0x38, 0x3C),
+	ILI9881C_COMMAND_INSTR(0x39, 0x00),
+	ILI9881C_COMMAND_INSTR(0x3a, 0x00),
+	ILI9881C_COMMAND_INSTR(0x3b, 0x00),
+	ILI9881C_COMMAND_INSTR(0x3c, 0x00),
+	ILI9881C_COMMAND_INSTR(0x3d, 0x00),
+	ILI9881C_COMMAND_INSTR(0x3e, 0x00),
+	ILI9881C_COMMAND_INSTR(0x3f, 0x00),
+	ILI9881C_COMMAND_INSTR(0x40, 0x00),
+	ILI9881C_COMMAND_INSTR(0x41, 0x00),
+	ILI9881C_COMMAND_INSTR(0x42, 0x00),
+	ILI9881C_COMMAND_INSTR(0x43, 0x00),
+	ILI9881C_COMMAND_INSTR(0x44, 0x00),
+	ILI9881C_COMMAND_INSTR(0x50, 0x01),
+	ILI9881C_COMMAND_INSTR(0x51, 0x23),
+	ILI9881C_COMMAND_INSTR(0x52, 0x45),
+	ILI9881C_COMMAND_INSTR(0x53, 0x67),
+	ILI9881C_COMMAND_INSTR(0x54, 0x89),
+	ILI9881C_COMMAND_INSTR(0x55, 0xab),
+	ILI9881C_COMMAND_INSTR(0x56, 0x01),
+	ILI9881C_COMMAND_INSTR(0x57, 0x23),
+	ILI9881C_COMMAND_INSTR(0x58, 0x45),
+	ILI9881C_COMMAND_INSTR(0x59, 0x67),
+	ILI9881C_COMMAND_INSTR(0x5a, 0x89),
+	ILI9881C_COMMAND_INSTR(0x5b, 0xab),
+	ILI9881C_COMMAND_INSTR(0x5c, 0xcd),
+	ILI9881C_COMMAND_INSTR(0x5d, 0xef),
+	ILI9881C_COMMAND_INSTR(0x5e, 0x11),
+	ILI9881C_COMMAND_INSTR(0x5f, 0x02),
+	ILI9881C_COMMAND_INSTR(0x60, 0x02),
+	ILI9881C_COMMAND_INSTR(0x61, 0x02),
+	ILI9881C_COMMAND_INSTR(0x62, 0x02),
+	ILI9881C_COMMAND_INSTR(0x63, 0x02),
+	ILI9881C_COMMAND_INSTR(0x64, 0x02),
+	ILI9881C_COMMAND_INSTR(0x65, 0x02),
+	ILI9881C_COMMAND_INSTR(0x66, 0x02),
+	ILI9881C_COMMAND_INSTR(0x67, 0x02),
+	ILI9881C_COMMAND_INSTR(0x68, 0x02),
+	ILI9881C_COMMAND_INSTR(0x69, 0x02),
+	ILI9881C_COMMAND_INSTR(0x6a, 0x0C),
+	ILI9881C_COMMAND_INSTR(0x6b, 0x02),
+	ILI9881C_COMMAND_INSTR(0x6c, 0x0F),
+	ILI9881C_COMMAND_INSTR(0x6d, 0x0E),
+	ILI9881C_COMMAND_INSTR(0x6e, 0x0D),
+	ILI9881C_COMMAND_INSTR(0x6f, 0x06),
+	ILI9881C_COMMAND_INSTR(0x70, 0x07),
+	ILI9881C_COMMAND_INSTR(0x71, 0x02),
+	ILI9881C_COMMAND_INSTR(0x72, 0x02),
+	ILI9881C_COMMAND_INSTR(0x73, 0x02),
+	ILI9881C_COMMAND_INSTR(0x74, 0x02),
+	ILI9881C_COMMAND_INSTR(0x75, 0x02),
+	ILI9881C_COMMAND_INSTR(0x76, 0x02),
+	ILI9881C_COMMAND_INSTR(0x77, 0x02),
+	ILI9881C_COMMAND_INSTR(0x78, 0x02),
+	ILI9881C_COMMAND_INSTR(0x79, 0x02),
+	ILI9881C_COMMAND_INSTR(0x7a, 0x02),
+	ILI9881C_COMMAND_INSTR(0x7b, 0x02),
+	ILI9881C_COMMAND_INSTR(0x7c, 0x02),
+	ILI9881C_COMMAND_INSTR(0x7d, 0x02),
+	ILI9881C_COMMAND_INSTR(0x7e, 0x02),
+	ILI9881C_COMMAND_INSTR(0x7f, 0x02),
+	ILI9881C_COMMAND_INSTR(0x80, 0x0C),
+	ILI9881C_COMMAND_INSTR(0x81, 0x02),
+	ILI9881C_COMMAND_INSTR(0x82, 0x0F),
+	ILI9881C_COMMAND_INSTR(0x83, 0x0E),
+	ILI9881C_COMMAND_INSTR(0x84, 0x0D),
+	ILI9881C_COMMAND_INSTR(0x85, 0x06),
+	ILI9881C_COMMAND_INSTR(0x86, 0x07),
+	ILI9881C_COMMAND_INSTR(0x87, 0x02),
+	ILI9881C_COMMAND_INSTR(0x88, 0x02),
+	ILI9881C_COMMAND_INSTR(0x89, 0x02),
+	ILI9881C_COMMAND_INSTR(0x8A, 0x02),
+	ILI9881C_SWITCH_PAGE_INSTR(4),
+	ILI9881C_COMMAND_INSTR(0x6C, 0x15),
+	ILI9881C_COMMAND_INSTR(0x6E, 0x22),
+	ILI9881C_COMMAND_INSTR(0x6F, 0x33),
+	ILI9881C_COMMAND_INSTR(0x3A, 0xA4),
+	ILI9881C_COMMAND_INSTR(0x8D, 0x0D),
+	ILI9881C_COMMAND_INSTR(0x87, 0xBA),
+	ILI9881C_COMMAND_INSTR(0x26, 0x76),
+	ILI9881C_COMMAND_INSTR(0xB2, 0xD1),
+	ILI9881C_SWITCH_PAGE_INSTR(1),
+	ILI9881C_COMMAND_INSTR(0x22, 0x0A),
+	ILI9881C_COMMAND_INSTR(0x53, 0xDC),
+	ILI9881C_COMMAND_INSTR(0x55, 0xA7),
+	ILI9881C_COMMAND_INSTR(0x50, 0x78),
+	ILI9881C_COMMAND_INSTR(0x51, 0x78),
+	ILI9881C_COMMAND_INSTR(0x31, 0x02),
+	ILI9881C_COMMAND_INSTR(0x60, 0x14),
+	ILI9881C_COMMAND_INSTR(0xA0, 0x2A),
+	ILI9881C_COMMAND_INSTR(0xA1, 0x39),
+	ILI9881C_COMMAND_INSTR(0xA2, 0x46),
+	ILI9881C_COMMAND_INSTR(0xA3, 0x0e),
+	ILI9881C_COMMAND_INSTR(0xA4, 0x12),
+	ILI9881C_COMMAND_INSTR(0xA5, 0x25),
+	ILI9881C_COMMAND_INSTR(0xA6, 0x19),
+	ILI9881C_COMMAND_INSTR(0xA7, 0x1d),
+	ILI9881C_COMMAND_INSTR(0xA8, 0xa6),
+	ILI9881C_COMMAND_INSTR(0xA9, 0x1C),
+	ILI9881C_COMMAND_INSTR(0xAA, 0x29),
+	ILI9881C_COMMAND_INSTR(0xAB, 0x85),
+	ILI9881C_COMMAND_INSTR(0xAC, 0x1C),
+	ILI9881C_COMMAND_INSTR(0xAD, 0x1B),
+	ILI9881C_COMMAND_INSTR(0xAE, 0x51),
+	ILI9881C_COMMAND_INSTR(0xAF, 0x22),
+	ILI9881C_COMMAND_INSTR(0xB0, 0x2d),
+	ILI9881C_COMMAND_INSTR(0xB1, 0x4f),
+	ILI9881C_COMMAND_INSTR(0xB2, 0x59),
+	ILI9881C_COMMAND_INSTR(0xB3, 0x3F),
+	ILI9881C_COMMAND_INSTR(0xC0, 0x2A),
+	ILI9881C_COMMAND_INSTR(0xC1, 0x3a),
+	ILI9881C_COMMAND_INSTR(0xC2, 0x45),
+	ILI9881C_COMMAND_INSTR(0xC3, 0x0e),
+	ILI9881C_COMMAND_INSTR(0xC4, 0x11),
+	ILI9881C_COMMAND_INSTR(0xC5, 0x24),
+	ILI9881C_COMMAND_INSTR(0xC6, 0x1a),
+	ILI9881C_COMMAND_INSTR(0xC7, 0x1c),
+	ILI9881C_COMMAND_INSTR(0xC8, 0xaa),
+	ILI9881C_COMMAND_INSTR(0xC9, 0x1C),
+	ILI9881C_COMMAND_INSTR(0xCA, 0x29),
+	ILI9881C_COMMAND_INSTR(0xCB, 0x96),
+	ILI9881C_COMMAND_INSTR(0xCC, 0x1C),
+	ILI9881C_COMMAND_INSTR(0xCD, 0x1B),
+	ILI9881C_COMMAND_INSTR(0xCE, 0x51),
+	ILI9881C_COMMAND_INSTR(0xCF, 0x22),
+	ILI9881C_COMMAND_INSTR(0xD0, 0x2b),
+	ILI9881C_COMMAND_INSTR(0xD1, 0x4b),
+	ILI9881C_COMMAND_INSTR(0xD2, 0x59),
+	ILI9881C_COMMAND_INSTR(0xD3, 0x3F),
+};
+
+static inline struct ili9881c *panel_to_ili9881c(struct drm_panel *panel)
+{
+	return container_of(panel, struct ili9881c, panel);
+}
+
+static int ili9881c_switch_page(struct ili9881c *ctx, u8 page)
+{
+	u8 buf[4] = { 0xff, 0x98, 0x81, page };
+	int ret;
+
+	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ili9881c_send_cmd_data(struct ili9881c *ctx, u8 cmd, u8 data)
+{
+	u8 buf[2] = { cmd, data };
+	int ret;
+
+	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ili9881c_prepare(struct drm_panel *panel)
+{
+	struct ili9881c *ctx = panel_to_ili9881c(panel);
+	unsigned int i;
+	int ret;
+
+	/* Power the panel */
+	gpiod_set_value(ctx->power, 1);
+	msleep(5);
+
+	/* And reset it */
+	gpiod_set_value(ctx->reset, 1);
+	msleep(20);
+
+	gpiod_set_value(ctx->reset, 0);
+	msleep(20);
+
+	for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
+		struct ili9881c_instr *instr = &ili9881c_init[i];
+
+		if (instr->op == ILI9881C_SWITCH_PAGE)
+			ret = ili9881c_switch_page(ctx, instr->arg.page);
+		else if (instr->op == ILI9881C_COMMAND)
+			ret = ili9881c_send_cmd_data(ctx, instr->arg.cmd.cmd,
+						      instr->arg.cmd.data);
+
+		if (ret)
+			return ret;
+	}
+
+	ret = ili9881c_switch_page(ctx, 0);
+	if (ret)
+		return ret;
+
+	ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+	if (ret)
+		return ret;
+
+	mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ili9881c_enable(struct drm_panel *panel)
+{
+	struct ili9881c *ctx = panel_to_ili9881c(panel);
+
+	msleep(120);
+
+	mipi_dsi_dcs_set_display_on(ctx->dsi);
+	backlight_enable(ctx->backlight);
+
+	return 0;
+}
+
+static int ili9881c_disable(struct drm_panel *panel)
+{
+	struct ili9881c *ctx = panel_to_ili9881c(panel);
+
+	backlight_disable(ctx->backlight);
+	return mipi_dsi_dcs_set_display_off(ctx->dsi);
+}
+
+static int ili9881c_unprepare(struct drm_panel *panel)
+{
+	struct ili9881c *ctx = panel_to_ili9881c(panel);
+
+	mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+	gpiod_set_value(ctx->power, 0);
+	gpiod_set_value(ctx->reset, 1);
+
+	return 0;
+}
+
+static const struct drm_display_mode default_mode = {
+	.clock		= 62000,
+	.vrefresh	= 60,
+
+	.hdisplay	= 720,
+	.hsync_start	= 720 + 10,
+	.hsync_end	= 720 + 10 + 20,
+	.htotal		= 720 + 10 + 20 + 30,
+
+	.vdisplay	= 1280,
+	.vsync_start	= 1280 + 10,
+	.vsync_end	= 1280 + 10 + 10,
+	.vtotal		= 1280 + 10 + 10 + 20,
+};
+
+static int ili9881c_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct ili9881c *ctx = panel_to_ili9881c(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &default_mode);
+	if (!mode) {
+		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
+			default_mode.hdisplay, default_mode.vdisplay,
+			default_mode.vrefresh);
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	panel->connector->display_info.width_mm = 62;
+	panel->connector->display_info.height_mm = 110;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ili9881c_funcs = {
+	.prepare	= ili9881c_prepare,
+	.unprepare	= ili9881c_unprepare,
+	.enable		= ili9881c_enable,
+	.disable	= ili9881c_disable,
+	.get_modes	= ili9881c_get_modes,
+};
+
+static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
+{
+	struct device_node *np;
+	struct ili9881c *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	mipi_dsi_set_drvdata(dsi, ctx);
+	ctx->dsi = dsi;
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = &dsi->dev;
+	ctx->panel.funcs = &ili9881c_funcs;
+
+	ctx->power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->power)) {
+		dev_err(&dsi->dev, "Couldn't get our power GPIO\n");
+		return PTR_ERR(ctx->power);
+	}
+
+	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset)) {
+		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
+		return PTR_ERR(ctx->reset);
+	}
+
+	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
+	if (np) {
+		ctx->backlight = of_find_backlight_by_node(np);
+		of_node_put(np);
+
+		if (!ctx->backlight)
+			return -EPROBE_DEFER;
+	}
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 4;
+
+	return mipi_dsi_attach(dsi);
+}
+
+static int ili9881c_dsi_remove(struct mipi_dsi_device *dsi)
+{
+	struct ili9881c *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&ctx->panel);
+
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	return 0;
+}
+
+static const struct of_device_id ili9881c_of_match[] = {
+	{ .compatible = "bananapi,lhr050h41" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ili9881c_of_match);
+
+static struct mipi_dsi_driver ili9881c_dsi_driver = {
+	.probe		= ili9881c_dsi_probe,
+	.remove		= ili9881c_dsi_remove,
+	.driver = {
+		.name		= "ili9881c-dsi",
+		.of_match_table	= ili9881c_of_match,
+	},
+};
+module_mipi_dsi_driver(ili9881c_dsi_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_DESCRIPTION("Ilitek ILI9881C Controller Driver");
+MODULE_LICENSE("GPL v2");
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display
  2018-05-04 15:23 [PATCH v4 0/3] drm/panel: Add Ilitek ILI9881c controller driver Maxime Ripard
  2018-05-04 15:23 ` [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
  2018-05-04 15:23 ` [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
@ 2018-05-04 15:23 ` Maxime Ripard
  2018-05-05  7:15   ` kbuild test robot
  2 siblings, 1 reply; 11+ messages in thread
From: Maxime Ripard @ 2018-05-04 15:23 UTC (permalink / raw)
  To: Thierry Reding, Chen-Yu Tsai, Maxime Ripard
  Cc: dri-devel, Gustavo Padovan, Daniel Vetter, Maarten Lankhorst,
	Sean Paul, linux-kernel

The BananaPi M2M has an optional 1280x720 DSI panel. Since that panel is
optional, we can only show a DT patch that would show how to enable it.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 39 +++++++++++++++++++++-
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
index eaf09666720d..30e710e94912 100644
--- a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
+++ b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
@@ -44,6 +44,7 @@
 #include "sun8i-a33.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "BananaPi M2 Magic";
@@ -81,6 +82,14 @@
 		};
 	};
 
+	pwm_bl: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <1 2 4 8 16 32 64 128 255>;
+		default-brightness-level = <8>;
+		enable-gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PG10 */
+	};
+
 	reg_vcc5v0: vcc5v0 {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc5v0";
@@ -120,6 +129,26 @@
 	status = "okay";
 };
 
+&de {
+	status = "okay";
+};
+
+&dphy {
+	status = "okay";
+};
+
+&dsi {
+	status = "okay";
+
+	panel@0 {
+		compatible = "bananapi,lhr050h41", "ilitek,ili9881c";
+		reg = <0>;
+		power-gpios = <&pio 1 7 GPIO_ACTIVE_HIGH>; /* PB07 */
+		reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL05 */
+		backlight = <&pwm_bl>;
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -179,6 +208,12 @@
 	status = "okay";
 };
 
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm0_pins>;
+	status = "okay";
+};
+
 &r_rsb {
 	status = "okay";
 
@@ -291,6 +326,10 @@
 	status = "okay";
 };
 
+&tcon0 {
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_b>;
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver
  2018-05-04 15:23 ` [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
@ 2018-05-05  3:48   ` kbuild test robot
  2018-05-05  7:15   ` kbuild test robot
  2018-05-11 15:54   ` Linus Walleij
  2 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-05-05  3:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: kbuild-all, Thierry Reding, Chen-Yu Tsai, Maxime Ripard,
	linux-kernel, dri-devel, Daniel Vetter

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

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-panel-Add-Ilitek-ILI9881c-controller-driver/20180505-104031
base:    
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.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
        make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c: In function 'ili9881c_prepare':
>> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:34: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      struct ili9881c_instr *instr = &ili9881c_init[i];
                                     ^

vim +/const +303 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c

   284	
   285	static int ili9881c_prepare(struct drm_panel *panel)
   286	{
   287		struct ili9881c *ctx = panel_to_ili9881c(panel);
   288		unsigned int i;
   289		int ret;
   290	
   291		/* Power the panel */
   292		gpiod_set_value(ctx->power, 1);
   293		msleep(5);
   294	
   295		/* And reset it */
   296		gpiod_set_value(ctx->reset, 1);
   297		msleep(20);
   298	
   299		gpiod_set_value(ctx->reset, 0);
   300		msleep(20);
   301	
   302		for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
 > 303			struct ili9881c_instr *instr = &ili9881c_init[i];
   304	
   305			if (instr->op == ILI9881C_SWITCH_PAGE)
   306				ret = ili9881c_switch_page(ctx, instr->arg.page);
   307			else if (instr->op == ILI9881C_COMMAND)
   308				ret = ili9881c_send_cmd_data(ctx, instr->arg.cmd.cmd,
   309							      instr->arg.cmd.data);
   310	
   311			if (ret)
   312				return ret;
   313		}
   314	
   315		ret = ili9881c_switch_page(ctx, 0);
   316		if (ret)
   317			return ret;
   318	
   319		ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
   320		if (ret)
   321			return ret;
   322	
   323		mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
   324		if (ret)
   325			return ret;
   326	
   327		return 0;
   328	}
   329	

---
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: 48088 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display
  2018-05-04 15:23 ` [PATCH v4 3/3] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard
@ 2018-05-05  7:15   ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-05-05  7:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: kbuild-all, Thierry Reding, Chen-Yu Tsai, Maxime Ripard,
	linux-kernel, dri-devel, Daniel Vetter

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

Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on ]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-panel-Add-Ilitek-ILI9881c-controller-driver/20180505-104031
base:    
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.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
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts:136.1-6 Label or path dphy not found
>> Error: arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts:140.1-5 Label or path dsi not found
>> FATAL ERROR: Syntax error parsing input tree

---
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: 23733 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver
  2018-05-04 15:23 ` [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
  2018-05-05  3:48   ` kbuild test robot
@ 2018-05-05  7:15   ` kbuild test robot
  2018-05-11 15:54   ` Linus Walleij
  2 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-05-05  7:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: kbuild-all, Thierry Reding, Chen-Yu Tsai, Maxime Ripard,
	linux-kernel, dri-devel, Daniel Vetter

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-panel-Add-Ilitek-ILI9881c-controller-driver/20180505-104031
base:    
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:62: sparse: incorrect type in initializer (different modifiers) @@    expected struct ili9881c_instr *instr @@    got structstruct ili9881c_instr *instr @@
   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:62:    expected struct ili9881c_instr *instr
   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:62:    got struct ili9881c_instr const *
   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c: In function 'ili9881c_prepare':
   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:34: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      struct ili9881c_instr *instr = &ili9881c_init[i];
                                     ^

vim +303 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c

   284	
   285	static int ili9881c_prepare(struct drm_panel *panel)
   286	{
   287		struct ili9881c *ctx = panel_to_ili9881c(panel);
   288		unsigned int i;
   289		int ret;
   290	
   291		/* Power the panel */
   292		gpiod_set_value(ctx->power, 1);
   293		msleep(5);
   294	
   295		/* And reset it */
   296		gpiod_set_value(ctx->reset, 1);
   297		msleep(20);
   298	
   299		gpiod_set_value(ctx->reset, 0);
   300		msleep(20);
   301	
   302		for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
 > 303			struct ili9881c_instr *instr = &ili9881c_init[i];
   304	
   305			if (instr->op == ILI9881C_SWITCH_PAGE)
   306				ret = ili9881c_switch_page(ctx, instr->arg.page);
   307			else if (instr->op == ILI9881C_COMMAND)
   308				ret = ili9881c_send_cmd_data(ctx, instr->arg.cmd.cmd,
   309							      instr->arg.cmd.data);
   310	
   311			if (ret)
   312				return ret;
   313		}
   314	
   315		ret = ili9881c_switch_page(ctx, 0);
   316		if (ret)
   317			return ret;
   318	
   319		ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
   320		if (ret)
   321			return ret;
   322	
   323		mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
   324		if (ret)
   325			return ret;
   326	
   327		return 0;
   328	}
   329	

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation
  2018-05-04 15:23 ` [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
@ 2018-05-11 15:40   ` Linus Walleij
  2018-05-11 15:55   ` Linus Walleij
  1 sibling, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2018-05-11 15:40 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Thierry Reding, Chen-Yu Tsai, linux-kernel,
	open list:DRM PANEL DRIVERS, Daniel Vetter

On Fri, May 4, 2018 at 5:23 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> The LHR050H41 from BananaPi is a 1280x700 4-lanes DSI panel based on the
> ILI9881c from Ilitek.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

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

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver
  2018-05-04 15:23 ` [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
  2018-05-05  3:48   ` kbuild test robot
  2018-05-05  7:15   ` kbuild test robot
@ 2018-05-11 15:54   ` Linus Walleij
  2018-05-29  9:23     ` Maxime Ripard
  2 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2018-05-11 15:54 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Thierry Reding, Chen-Yu Tsai, open list:DRM PANEL DRIVERS,
	Gustavo Padovan, Daniel Vetter, Maarten Lankhorst, Sean Paul,
	linux-kernel

Hi Maxime,

sorry that noone had much time to look at the driver.
But I can help out, hopefully.

On Fri, May 4, 2018 at 5:23 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:

> The LHR050H41 panel is the panel shipped with the BananaPi M2-Magic, and is
> based on the Ilitek ILI9881c Controller. Add a driver for it, modelled
> after the other Ilitek controller drivers.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

Nice, I have some experience with those.

> +config DRM_PANEL_ILITEK_ILI9881C
> +       tristate "Ilitek ILI9881C-based panels"
> +       depends on OF
> +       depends on DRM_MIPI_DSI
> +       depends on BACKLIGHT_CLASS_DEVICE

If it absolutely needs DRM_MIPI_DSI and
BACKLIGHT_CLASS_DEVICE it maybe you should
be more helpful to the user to just select it?

Depending on OF is fine, that is more of a platform
property.

> +struct ili9881c {
> +       struct drm_panel        panel;
> +       struct mipi_dsi_device  *dsi;
> +
> +       struct backlight_device *backlight;
> +       struct gpio_desc        *power;

Should this not be modeled using a fixed regulator instead?

> +       struct gpio_desc        *reset;
> +};

> +enum ili9881c_op {
> +       ILI9881C_SWITCH_PAGE,
> +       ILI9881C_COMMAND,
> +};
> +
> +struct ili9881c_instr {
> +       enum ili9881c_op        op;
> +
> +       union arg {
> +               struct cmd {
> +                       u8      cmd;
> +                       u8      data;
> +               } cmd;
> +               u8      page;
> +       } arg;
> +};
> +
> +#define ILI9881C_SWITCH_PAGE_INSTR(_page)      \
> +       {                                       \
> +               .op = ILI9881C_SWITCH_PAGE,     \
> +               .arg = {                        \
> +                       .page = (_page),        \
> +               },                              \
> +       }
> +
> +#define ILI9881C_COMMAND_INSTR(_cmd, _data)            \
> +       {                                               \
> +               .op = ILI9881C_COMMAND,         \
> +               .arg = {                                \
> +                       .cmd = {                        \
> +                               .cmd = (_cmd),          \
> +                               .data = (_data),        \
> +                       },                              \
> +               },                                      \
> +       }

That looks clever.

> +static const struct ili9881c_instr ili9881c_init[] = {
> +       ILI9881C_SWITCH_PAGE_INSTR(3),
> +       ILI9881C_COMMAND_INSTR(0x01, 0x00),
> +       ILI9881C_COMMAND_INSTR(0x02, 0x00),
> +       ILI9881C_COMMAND_INSTR(0x03, 0x73),
> +       ILI9881C_COMMAND_INSTR(0x04, 0x03),
> +       ILI9881C_COMMAND_INSTR(0x05, 0x00),
> +       ILI9881C_COMMAND_INSTR(0x06, 0x06),
> +       ILI9881C_COMMAND_INSTR(0x07, 0x06),
(...)

This is a bit hard to understand to say the least. If this comes from
a datasheet some more elaborate construction of the commands
would be nice, maybe using some #defines?

If this just comes from some code drop or reverse engineering,
OK bad luck, I can live with it :)

It looks a bit like you're uploading a small firmware.

> +static int ili9881c_switch_page(struct ili9881c *ctx, u8 page)
> +{
> +       u8 buf[4] = { 0xff, 0x98, 0x81, page };

That is also a bit unclear wrt what is going on.

> +static int ili9881c_prepare(struct drm_panel *panel)
> +{
> +       struct ili9881c *ctx = panel_to_ili9881c(panel);
> +       unsigned int i;
> +       int ret;
> +
> +       /* Power the panel */
> +       gpiod_set_value(ctx->power, 1);
> +       msleep(5);

So isn't this a fixed regulator using a GPIO?

> +static const struct drm_display_mode default_mode = {
> +       .clock          = 62000,
> +       .vrefresh       = 60,
> +
> +       .hdisplay       = 720,
> +       .hsync_start    = 720 + 10,
> +       .hsync_end      = 720 + 10 + 20,
> +       .htotal         = 720 + 10 + 20 + 30,
> +
> +       .vdisplay       = 1280,
> +       .vsync_start    = 1280 + 10,
> +       .vsync_end      = 1280 + 10 + 10,
> +       .vtotal         = 1280 + 10 + 10 + 20,
> +};

Is that the default mode for this Ilitek device or just for the
BananaPi? If it is the latter it should be named
bananapi_default_mode or something so as not to confuse
the next user of this driver.

> +       ctx->power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW);
> +       if (IS_ERR(ctx->power)) {
> +               dev_err(&dsi->dev, "Couldn't get our power GPIO\n");
> +               return PTR_ERR(ctx->power);
> +       }

So I'm a bit sceptical about this one.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation
  2018-05-04 15:23 ` [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
  2018-05-11 15:40   ` Linus Walleij
@ 2018-05-11 15:55   ` Linus Walleij
  1 sibling, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2018-05-11 15:55 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Thierry Reding, Chen-Yu Tsai, linux-kernel,
	open list:DRM PANEL DRIVERS, Daniel Vetter

On Fri, May 4, 2018 at 5:23 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:

> The LHR050H41 from BananaPi is a 1280x700 4-lanes DSI panel based on the
> ILI9881c from Ilitek.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
(...)

Bah, now I see this:

> +  - power-gpios: a GPIO phandle for the power pin

That should probably be a fixed regulator backed by a GPIO
instead. Especially if you know which voltage it has. So
one more layer of abstraction, sorry.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver
  2018-05-11 15:54   ` Linus Walleij
@ 2018-05-29  9:23     ` Maxime Ripard
  0 siblings, 0 replies; 11+ messages in thread
From: Maxime Ripard @ 2018-05-29  9:23 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Thierry Reding, Chen-Yu Tsai, open list:DRM PANEL DRIVERS,
	Gustavo Padovan, Daniel Vetter, Maarten Lankhorst, Sean Paul,
	linux-kernel

Hi Linus,

On Fri, May 11, 2018 at 05:54:27PM +0200, Linus Walleij wrote:
> Hi Maxime,
> 
> sorry that noone had much time to look at the driver.
> But I can help out, hopefully.

Thanks for your feedback :)

> On Fri, May 4, 2018 at 5:23 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> 
> > The LHR050H41 panel is the panel shipped with the BananaPi M2-Magic, and is
> > based on the Ilitek ILI9881c Controller. Add a driver for it, modelled
> > after the other Ilitek controller drivers.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> 
> Nice, I have some experience with those.
> 
> > +config DRM_PANEL_ILITEK_ILI9881C
> > +       tristate "Ilitek ILI9881C-based panels"
> > +       depends on OF
> > +       depends on DRM_MIPI_DSI
> > +       depends on BACKLIGHT_CLASS_DEVICE
> 
> If it absolutely needs DRM_MIPI_DSI and
> BACKLIGHT_CLASS_DEVICE it maybe you should
> be more helpful to the user to just select it?
> 
> Depending on OF is fine, that is more of a platform
> property.

All the other panels in this file seems to be using a depends on for
these two, so I'd rather remain consistent on this.

> > +struct ili9881c {
> > +       struct drm_panel        panel;
> > +       struct mipi_dsi_device  *dsi;
> > +
> > +       struct backlight_device *backlight;
> > +       struct gpio_desc        *power;
> 
> Should this not be modeled using a fixed regulator instead?

Probably, yes. On my board it's a GPIO that goes through the
connector, but it seems like the controller itself takes a regulator,
so there's probably a GPIO-controlled regulator on the display PCB
somewhere. I'll change it.

> > +static const struct ili9881c_instr ili9881c_init[] = {
> > +       ILI9881C_SWITCH_PAGE_INSTR(3),
> > +       ILI9881C_COMMAND_INSTR(0x01, 0x00),
> > +       ILI9881C_COMMAND_INSTR(0x02, 0x00),
> > +       ILI9881C_COMMAND_INSTR(0x03, 0x73),
> > +       ILI9881C_COMMAND_INSTR(0x04, 0x03),
> > +       ILI9881C_COMMAND_INSTR(0x05, 0x00),
> > +       ILI9881C_COMMAND_INSTR(0x06, 0x06),
> > +       ILI9881C_COMMAND_INSTR(0x07, 0x06),
> (...)
> 
> This is a bit hard to understand to say the least.

I know :/

> If this comes from a datasheet some more elaborate construction of
> the commands would be nice, maybe using some #defines?
> 
> If this just comes from some code drop or reverse engineering,
> OK bad luck, I can live with it :)

This comes from a code drop (or rather, an obscure initialisation
sequence coming straight from the vendor without any documentation or
comment associated to it).

> It looks a bit like you're uploading a small firmware.
> 
> > +static int ili9881c_switch_page(struct ili9881c *ctx, u8 page)
> > +{
> > +       u8 buf[4] = { 0xff, 0x98, 0x81, page };
> 
> That is also a bit unclear wrt what is going on.

My understanding is that the controller maps some registers (that
might be accessible through other buses if the controller is setup to
use something other than DCS) to DCS commands. Since there's more
commands than DCS would allow, it's setup in several pages, with each
pages having its own set of commands, and the page 0 accepting the
usual standard DCS commands.

It really doesn't look to me like a firmware, but just a poorly
documented initialisation sequence.. I'll add a comment

> > +static const struct drm_display_mode default_mode = {
> > +       .clock          = 62000,
> > +       .vrefresh       = 60,
> > +
> > +       .hdisplay       = 720,
> > +       .hsync_start    = 720 + 10,
> > +       .hsync_end      = 720 + 10 + 20,
> > +       .htotal         = 720 + 10 + 20 + 30,
> > +
> > +       .vdisplay       = 1280,
> > +       .vsync_start    = 1280 + 10,
> > +       .vsync_end      = 1280 + 10 + 10,
> > +       .vtotal         = 1280 + 10 + 10 + 20,
> > +};
> 
> Is that the default mode for this Ilitek device or just for the
> BananaPi? If it is the latter it should be named
> bananapi_default_mode or something so as not to confuse
> the next user of this driver.

I'll do it, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-05-29  9:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 15:23 [PATCH v4 0/3] drm/panel: Add Ilitek ILI9881c controller driver Maxime Ripard
2018-05-04 15:23 ` [PATCH v4 1/3] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
2018-05-11 15:40   ` Linus Walleij
2018-05-11 15:55   ` Linus Walleij
2018-05-04 15:23 ` [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
2018-05-05  3:48   ` kbuild test robot
2018-05-05  7:15   ` kbuild test robot
2018-05-11 15:54   ` Linus Walleij
2018-05-29  9:23     ` Maxime Ripard
2018-05-04 15:23 ` [PATCH v4 3/3] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard
2018-05-05  7:15   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).