[PATCH v3 0/2] Add DSI panel driver for Raydium RM67191

[PATCH v3 0/2] Add DSI panel driver for Raydium RM67191

[Robert Chiras]

This patch-set contains the DRM panel driver and dt-bindings documentation
for the DSI driven panel: Raydium RM67191.

v3:
- Added myself to MAINTAINERS for this driver (sam)
- Removed display-timings property (fabio)
- Fixed dt description (sam)
- Re-arranged calls inside get_modes function (sam)
- Changed ifdefs with _maybe_unused for suspend/resume functions (sam)
- Collected Reviewed-by from Sam

v2:
- Fixed 'reset-gpio' to 'reset-gpios' property naming (fabio)
- Changed the state of the reset gpio to active low and fixed how it is
  handled in driver (fabio)
- Fixed copyright statement (daniel)
- Reordered includes (sam)
- Added defines for panel specific color formats (fabio)
- Removed unnecessary tests in enable and unprepare (sam)
- Removed the unnecessary backlight write in enable (sam)

Robert Chiras (2):
  dt-bindings: display: panel: Add support for Raydium RM67191 panel
  drm/panel: Add support for Raydium RM67191 panel driver

 .../bindings/display/panel/raydium,rm67191.txt     |  39 ++
 MAINTAINERS                                        |   6 +
 drivers/gpu/drm/panel/Kconfig                      |   9 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-raydium-rm67191.c      | 690 +++++++++++++++++++++
 5 files changed, 745 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
 create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c

-- 
2.7.4

[PATCH v3 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

[Robert Chiras]

Add dt-bindings documentation for Raydium RM67191 DSI panel.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 .../bindings/display/panel/raydium,rm67191.txt     | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt

diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
new file mode 100644
index 0000000..52af272
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
@@ -0,0 +1,39 @@
+Raydium RM67171 OLED LCD panel with MIPI-DSI protocol
+
+Required properties:
+- compatible: 		"raydium,rm67191"
+- reg:			virtual channel for MIPI-DSI protocol
+			must be <0>
+- dsi-lanes:		number of DSI lanes to be used
+			must be <3> or <4>
+- port: 		input port node with endpoint definition as
+			defined in Documentation/devicetree/bindings/graph.txt;
+			the input port should be connected to a MIPI-DSI device
+			driver
+
+Optional properties:
+- reset-gpios:		a GPIO spec for the RST_B GPIO pin
+- width-mm:		see panel-common.txt
+- height-mm:		see panel-common.txt
+- video-mode:		0 - burst-mode
+			1 - non-burst with sync event
+			2 - non-burst with sync pulse
+
+Example:
+
+	panel@0 {
+		compatible = "raydium,rm67191";
+		reg = <0>;
+		pinctrl-0 = <&pinctrl_mipi_dsi_0_1_en>;
+		pinctrl-names = "default";
+		reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
+		dsi-lanes = <4>;
+		width-mm = <68>;
+		height-mm = <121>;
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&mipi_out>;
+			};
+		};
+	};
-- 
2.7.4

[PATCH v3 2/2] drm/panel: Add support for Raydium RM67191 panel driver

[Robert Chiras]

This patch adds Raydium RM67191 TFT LCD panel driver (MIPI-DSI
protocol).

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-raydium-rm67191.c | 690 ++++++++++++++++++++++++++
 4 files changed, 706 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2f487..cd93030e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5089,6 +5089,12 @@ S:	Maintained
 F:	drivers/gpu/drm/qxl/
 F:	include/uapi/drm/qxl_drm.h
 
+DRM DRIVER FOR RAYDIUM RM67191 PANELS
+M:	Robert Chiras <robert.chiras@nxp.com>
+S:	Maintained
+F:	drivers/gpu/drm/panel/panel-raydium-rm67191.c
+F:	Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
+
 DRM DRIVER FOR RAGE 128 VIDEO CARDS
 S:	Orphan / Obsolete
 F:	drivers/gpu/drm/r128/
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index d9d931a..8be1ac1 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -159,6 +159,15 @@ config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN
 	  Pi 7" Touchscreen.  To compile this driver as a module,
 	  choose M here.
 
+config DRM_PANEL_RAYDIUM_RM67191
+	tristate "Raydium RM67191 FHD 1080x1920 DSI video mode panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for Raydium RM67191 FHD
+	  (1080x1920) DSI panel.
+
 config DRM_PANEL_RAYDIUM_RM68200
 	tristate "Raydium RM68200 720x1280 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index fb0cb3a..1fc0f68 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
 obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
 obj-$(CONFIG_DRM_PANEL_ROCKTECH_JH057N00900) += panel-rocktech-jh057n00900.o
 obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
new file mode 100644
index 0000000..c4d7dfd
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
@@ -0,0 +1,690 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Raydium RM67191 MIPI-DSI panel driver
+ *
+ * Copyright 2019 NXP
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+/* Panel specific color-format bits */
+#define COL_FMT_16BPP 0x55
+#define COL_FMT_18BPP 0x66
+#define COL_FMT_24BPP 0x77
+
+/* Write Manufacture Command Set Control */
+#define WRMAUCCTR 0xFE
+
+/* Manufacturer Command Set pages (CMD2) */
+struct cmd_set_entry {
+	u8 cmd;
+	u8 param;
+};
+
+/*
+ * There is no description in the Reference Manual about these commands.
+ * We received them from vendor, so just use them as is.
+ */
+static const struct cmd_set_entry manufacturer_cmd_set[] = {
+	{0xFE, 0x0B},
+	{0x28, 0x40},
+	{0x29, 0x4F},
+	{0xFE, 0x0E},
+	{0x4B, 0x00},
+	{0x4C, 0x0F},
+	{0x4D, 0x20},
+	{0x4E, 0x40},
+	{0x4F, 0x60},
+	{0x50, 0xA0},
+	{0x51, 0xC0},
+	{0x52, 0xE0},
+	{0x53, 0xFF},
+	{0xFE, 0x0D},
+	{0x18, 0x08},
+	{0x42, 0x00},
+	{0x08, 0x41},
+	{0x46, 0x02},
+	{0x72, 0x09},
+	{0xFE, 0x0A},
+	{0x24, 0x17},
+	{0x04, 0x07},
+	{0x1A, 0x0C},
+	{0x0F, 0x44},
+	{0xFE, 0x04},
+	{0x00, 0x0C},
+	{0x05, 0x08},
+	{0x06, 0x08},
+	{0x08, 0x08},
+	{0x09, 0x08},
+	{0x0A, 0xE6},
+	{0x0B, 0x8C},
+	{0x1A, 0x12},
+	{0x1E, 0xE0},
+	{0x29, 0x93},
+	{0x2A, 0x93},
+	{0x2F, 0x02},
+	{0x31, 0x02},
+	{0x33, 0x05},
+	{0x37, 0x2D},
+	{0x38, 0x2D},
+	{0x3A, 0x1E},
+	{0x3B, 0x1E},
+	{0x3D, 0x27},
+	{0x3F, 0x80},
+	{0x40, 0x40},
+	{0x41, 0xE0},
+	{0x4F, 0x2F},
+	{0x50, 0x1E},
+	{0xFE, 0x06},
+	{0x00, 0xCC},
+	{0x05, 0x05},
+	{0x07, 0xA2},
+	{0x08, 0xCC},
+	{0x0D, 0x03},
+	{0x0F, 0xA2},
+	{0x32, 0xCC},
+	{0x37, 0x05},
+	{0x39, 0x83},
+	{0x3A, 0xCC},
+	{0x41, 0x04},
+	{0x43, 0x83},
+	{0x44, 0xCC},
+	{0x49, 0x05},
+	{0x4B, 0xA2},
+	{0x4C, 0xCC},
+	{0x51, 0x03},
+	{0x53, 0xA2},
+	{0x75, 0xCC},
+	{0x7A, 0x03},
+	{0x7C, 0x83},
+	{0x7D, 0xCC},
+	{0x82, 0x02},
+	{0x84, 0x83},
+	{0x85, 0xEC},
+	{0x86, 0x0F},
+	{0x87, 0xFF},
+	{0x88, 0x00},
+	{0x8A, 0x02},
+	{0x8C, 0xA2},
+	{0x8D, 0xEA},
+	{0x8E, 0x01},
+	{0x8F, 0xE8},
+	{0xFE, 0x06},
+	{0x90, 0x0A},
+	{0x92, 0x06},
+	{0x93, 0xA0},
+	{0x94, 0xA8},
+	{0x95, 0xEC},
+	{0x96, 0x0F},
+	{0x97, 0xFF},
+	{0x98, 0x00},
+	{0x9A, 0x02},
+	{0x9C, 0xA2},
+	{0xAC, 0x04},
+	{0xFE, 0x06},
+	{0xB1, 0x12},
+	{0xB2, 0x17},
+	{0xB3, 0x17},
+	{0xB4, 0x17},
+	{0xB5, 0x17},
+	{0xB6, 0x11},
+	{0xB7, 0x08},
+	{0xB8, 0x09},
+	{0xB9, 0x06},
+	{0xBA, 0x07},
+	{0xBB, 0x17},
+	{0xBC, 0x17},
+	{0xBD, 0x17},
+	{0xBE, 0x17},
+	{0xBF, 0x17},
+	{0xC0, 0x17},
+	{0xC1, 0x17},
+	{0xC2, 0x17},
+	{0xC3, 0x17},
+	{0xC4, 0x0F},
+	{0xC5, 0x0E},
+	{0xC6, 0x00},
+	{0xC7, 0x01},
+	{0xC8, 0x10},
+	{0xFE, 0x06},
+	{0x95, 0xEC},
+	{0x8D, 0xEE},
+	{0x44, 0xEC},
+	{0x4C, 0xEC},
+	{0x32, 0xEC},
+	{0x3A, 0xEC},
+	{0x7D, 0xEC},
+	{0x75, 0xEC},
+	{0x00, 0xEC},
+	{0x08, 0xEC},
+	{0x85, 0xEC},
+	{0xA6, 0x21},
+	{0xA7, 0x05},
+	{0xA9, 0x06},
+	{0x82, 0x06},
+	{0x41, 0x06},
+	{0x7A, 0x07},
+	{0x37, 0x07},
+	{0x05, 0x06},
+	{0x49, 0x06},
+	{0x0D, 0x04},
+	{0x51, 0x04},
+};
+
+static const u32 rad_bus_formats[] = {
+	MEDIA_BUS_FMT_RGB888_1X24,
+	MEDIA_BUS_FMT_RGB666_1X18,
+	MEDIA_BUS_FMT_RGB565_1X16,
+};
+
+struct rad_panel {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+
+	struct gpio_desc *reset;
+	struct backlight_device *backlight;
+
+	bool prepared;
+	bool enabled;
+
+	struct videomode vm;
+	u32 width_mm;
+	u32 height_mm;
+};
+
+static inline struct rad_panel *to_rad_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct rad_panel, panel);
+}
+
+static int rad_panel_push_cmd_list(struct mipi_dsi_device *dsi)
+{
+	size_t i;
+	size_t count = ARRAY_SIZE(manufacturer_cmd_set);
+	int ret = 0;
+
+	for (i = 0; i < count; i++) {
+		const struct cmd_set_entry *entry = &manufacturer_cmd_set[i];
+		u8 buffer[2] = { entry->cmd, entry->param };
+
+		ret = mipi_dsi_generic_write(dsi, &buffer, sizeof(buffer));
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+};
+
+static int color_format_from_dsi_format(enum mipi_dsi_pixel_format format)
+{
+	switch (format) {
+	case MIPI_DSI_FMT_RGB565:
+		return COL_FMT_16BPP;
+	case MIPI_DSI_FMT_RGB666:
+	case MIPI_DSI_FMT_RGB666_PACKED:
+		return COL_FMT_18BPP;
+	case MIPI_DSI_FMT_RGB888:
+		return COL_FMT_24BPP;
+	default:
+		return COL_FMT_24BPP; /* for backward compatibility */
+	}
+};
+
+static int rad_panel_prepare(struct drm_panel *panel)
+{
+	struct rad_panel *rad = to_rad_panel(panel);
+
+	if (rad->prepared)
+		return 0;
+
+	if (rad->reset) {
+		gpiod_set_value_cansleep(rad->reset, 1);
+		usleep_range(3000, 5000);
+		gpiod_set_value_cansleep(rad->reset, 0);
+		usleep_range(18000, 20000);
+	}
+
+	rad->prepared = true;
+
+	return 0;
+}
+
+static int rad_panel_unprepare(struct drm_panel *panel)
+{
+	struct rad_panel *rad = to_rad_panel(panel);
+
+	if (!rad->prepared)
+		return 0;
+
+	/*
+	 * Right after asserting the reset, we need to release it, so that the
+	 * touch driver can have an active connection with the touch controller
+	 * even after the display is turned off.
+	 */
+	if (rad->reset) {
+		gpiod_set_value_cansleep(rad->reset, 1);
+		usleep_range(15000, 17000);
+		gpiod_set_value_cansleep(rad->reset, 0);
+	}
+
+	rad->prepared = false;
+
+	return 0;
+}
+
+static int rad_panel_enable(struct drm_panel *panel)
+{
+	struct rad_panel *rad = to_rad_panel(panel);
+	struct mipi_dsi_device *dsi = rad->dsi;
+	struct device *dev = &dsi->dev;
+	int color_format = color_format_from_dsi_format(dsi->format);
+	int ret;
+
+	if (rad->enabled)
+		return 0;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	ret = rad_panel_push_cmd_list(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to send MCS (%d)\n", ret);
+		goto fail;
+	}
+
+	/* Select User Command Set table (CMD1) */
+	ret = mipi_dsi_generic_write(dsi, (u8[]){ WRMAUCCTR, 0x00 }, 2);
+	if (ret < 0)
+		goto fail;
+
+	/* Software reset */
+	ret = mipi_dsi_dcs_soft_reset(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to do Software Reset (%d)\n", ret);
+		goto fail;
+	}
+
+	usleep_range(15000, 17000);
+
+	/* Set DSI mode */
+	ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xC2, 0x0B }, 2);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to set DSI mode (%d)\n", ret);
+		goto fail;
+	}
+	/* Set tear ON */
+	ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to set tear ON (%d)\n", ret);
+		goto fail;
+	}
+	/* Set tear scanline */
+	ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0x380);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to set tear scanline (%d)\n", ret);
+		goto fail;
+	}
+	/* Set pixel format */
+	ret = mipi_dsi_dcs_set_pixel_format(dsi, color_format);
+	DRM_DEV_DEBUG_DRIVER(dev, "Interface color format set to 0x%x\n",
+			     color_format);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to set pixel format (%d)\n", ret);
+		goto fail;
+	}
+	/* Exit sleep mode */
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to exit sleep mode (%d)\n", ret);
+		goto fail;
+	}
+
+	usleep_range(5000, 7000);
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to set display ON (%d)\n", ret);
+		goto fail;
+	}
+
+	backlight_enable(rad->backlight);
+
+	rad->enabled = true;
+
+	return 0;
+
+fail:
+	if (rad->reset)
+		gpiod_set_value_cansleep(rad->reset, 1);
+
+	return ret;
+}
+
+static int rad_panel_disable(struct drm_panel *panel)
+{
+	struct rad_panel *rad = to_rad_panel(panel);
+	struct mipi_dsi_device *dsi = rad->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (!rad->enabled)
+		return 0;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	backlight_disable(rad->backlight);
+
+	usleep_range(10000, 12000);
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to set display OFF (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(5000, 10000);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev, "Failed to enter sleep mode (%d)\n", ret);
+		return ret;
+	}
+
+	rad->enabled = false;
+
+	return 0;
+}
+
+static int rad_panel_get_modes(struct drm_panel *panel)
+{
+	struct rad_panel *rad = to_rad_panel(panel);
+	struct device *dev = &rad->dsi->dev;
+	struct drm_connector *connector = panel->connector;
+	struct drm_display_mode *mode;
+	u32 *bus_flags = &connector->display_info.bus_flags;
+
+	mode = drm_mode_create(connector->dev);
+	if (!mode) {
+		DRM_DEV_ERROR(dev, "Failed to create display mode!\n");
+		return 0;
+	}
+
+	drm_display_mode_from_videomode(&rad->vm, mode);
+	mode->width_mm = rad->width_mm;
+	mode->height_mm = rad->height_mm;
+	connector->display_info.width_mm = rad->width_mm;
+	connector->display_info.height_mm = rad->height_mm;
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+
+	if (rad->vm.flags & DISPLAY_FLAGS_DE_HIGH)
+		*bus_flags |= DRM_BUS_FLAG_DE_HIGH;
+	if (rad->vm.flags & DISPLAY_FLAGS_DE_LOW)
+		*bus_flags |= DRM_BUS_FLAG_DE_LOW;
+	if (rad->vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+		*bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+	if (rad->vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		*bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
+
+	drm_mode_probed_add(panel->connector, mode);
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 rad_bus_formats,
+					 ARRAY_SIZE(rad_bus_formats));
+	return 1;
+}
+
+static int rad_bl_get_brightness(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	struct rad_panel *rad = mipi_dsi_get_drvdata(dsi);
+	struct device *dev = &dsi->dev;
+	u16 brightness;
+	int ret;
+
+	if (!rad->prepared)
+		return 0;
+
+	DRM_DEV_DEBUG_DRIVER(dev, "\n");
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
+	if (ret < 0)
+		return ret;
+
+	bl->props.brightness = brightness;
+
+	return brightness & 0xff;
+}
+
+static int rad_bl_update_status(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	struct rad_panel *rad = mipi_dsi_get_drvdata(dsi);
+	struct device *dev = &dsi->dev;
+	int ret = 0;
+
+	if (!rad->prepared)
+		return 0;
+
+	DRM_DEV_DEBUG_DRIVER(dev, "New brightness: %d\n", bl->props.brightness);
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static const struct backlight_ops rad_bl_ops = {
+	.update_status = rad_bl_update_status,
+	.get_brightness = rad_bl_get_brightness,
+};
+
+static const struct drm_panel_funcs rad_panel_funcs = {
+	.prepare = rad_panel_prepare,
+	.unprepare = rad_panel_unprepare,
+	.enable = rad_panel_enable,
+	.disable = rad_panel_disable,
+	.get_modes = rad_panel_get_modes,
+};
+
+static const struct display_timing rad_default_timing = {
+	.pixelclock = { 132000000, 132000000, 132000000 },
+	.hactive = { 1080, 1080, 1080 },
+	.hfront_porch = { 20, 20, 20 },
+	.hsync_len = { 2, 2, 2 },
+	.hback_porch = { 34, 34, 34 },
+	.vactive = { 1920, 1920, 1920 },
+	.vfront_porch = { 10, 10, 10 },
+	.vsync_len = { 2, 2, 2 },
+	.vback_porch = { 4, 4, 4 },
+	.flags = DISPLAY_FLAGS_HSYNC_LOW |
+		 DISPLAY_FLAGS_VSYNC_LOW |
+		 DISPLAY_FLAGS_DE_LOW |
+		 DISPLAY_FLAGS_PIXDATA_NEGEDGE,
+};
+
+static int rad_panel_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct device_node *np = dev->of_node;
+	struct rad_panel *panel;
+	struct backlight_properties bl_props;
+	int ret;
+	u32 video_mode;
+
+	panel = devm_kzalloc(&dsi->dev, sizeof(*panel), GFP_KERNEL);
+	if (!panel)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, panel);
+
+	panel->dsi = dsi;
+
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags =  MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO |
+			   MIPI_DSI_CLOCK_NON_CONTINUOUS;
+
+	ret = of_property_read_u32(np, "video-mode", &video_mode);
+	if (!ret) {
+		switch (video_mode) {
+		case 0:
+			/* burst mode */
+			dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_BURST;
+			break;
+		case 1:
+			/* non-burst mode with sync event */
+			break;
+		case 2:
+			/* non-burst mode with sync pulse */
+			dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+			break;
+		default:
+			dev_warn(dev, "invalid video mode %d\n", video_mode);
+			break;
+		}
+	}
+
+	ret = of_property_read_u32(np, "dsi-lanes", &dsi->lanes);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get dsi-lanes property (%d)\n", ret);
+		return ret;
+	}
+
+	videomode_from_timing(&rad_default_timing, &panel->vm);
+
+	of_property_read_u32(np, "width-mm", &panel->width_mm);
+	of_property_read_u32(np, "height-mm", &panel->height_mm);
+
+	panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+
+	if (IS_ERR(panel->reset))
+		panel->reset = NULL;
+	else
+		gpiod_set_value_cansleep(panel->reset, 1);
+
+	memset(&bl_props, 0, sizeof(bl_props));
+	bl_props.type = BACKLIGHT_RAW;
+	bl_props.brightness = 255;
+	bl_props.max_brightness = 255;
+
+	panel->backlight = devm_backlight_device_register(dev, dev_name(dev),
+							  dev, dsi,
+							  &rad_bl_ops,
+							  &bl_props);
+	if (IS_ERR(panel->backlight)) {
+		ret = PTR_ERR(panel->backlight);
+		dev_err(dev, "Failed to register backlight (%d)\n", ret);
+		return ret;
+	}
+
+	drm_panel_init(&panel->panel);
+	panel->panel.funcs = &rad_panel_funcs;
+	panel->panel.dev = dev;
+	dev_set_drvdata(dev, panel);
+
+	ret = drm_panel_add(&panel->panel);
+
+	if (ret < 0)
+		return ret;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0)
+		drm_panel_remove(&panel->panel);
+
+	return ret;
+}
+
+static int rad_panel_remove(struct mipi_dsi_device *dsi)
+{
+	struct rad_panel *rad = mipi_dsi_get_drvdata(dsi);
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		DRM_DEV_ERROR(dev, "Failed to detach from host (%d)\n",
+			      ret);
+
+	drm_panel_remove(&rad->panel);
+
+	return 0;
+}
+
+static void rad_panel_shutdown(struct mipi_dsi_device *dsi)
+{
+	struct rad_panel *rad = mipi_dsi_get_drvdata(dsi);
+
+	rad_panel_disable(&rad->panel);
+	rad_panel_unprepare(&rad->panel);
+}
+
+static int __maybe_unused rad_panel_suspend(struct device *dev)
+{
+	struct rad_panel *rad = dev_get_drvdata(dev);
+
+	if (!rad->reset)
+		return 0;
+
+	devm_gpiod_put(dev, rad->reset);
+	rad->reset = NULL;
+
+	return 0;
+}
+
+static int __maybe_unused rad_panel_resume(struct device *dev)
+{
+	struct rad_panel *rad = dev_get_drvdata(dev);
+
+	if (rad->reset)
+		return 0;
+
+	rad->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(rad->reset))
+		rad->reset = NULL;
+
+	return PTR_ERR_OR_ZERO(rad->reset);
+}
+
+static const struct dev_pm_ops rad_pm_ops = {
+	SET_RUNTIME_PM_OPS(rad_panel_suspend, rad_panel_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(rad_panel_suspend, rad_panel_resume)
+};
+
+static const struct of_device_id rad_of_match[] = {
+	{ .compatible = "raydium,rm67191", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rad_of_match);
+
+static struct mipi_dsi_driver rad_panel_driver = {
+	.driver = {
+		.name = "panel-raydium-rm67191",
+		.of_match_table = rad_of_match,
+		.pm	= &rad_pm_ops,
+	},
+	.probe = rad_panel_probe,
+	.remove = rad_panel_remove,
+	.shutdown = rad_panel_shutdown,
+};
+module_mipi_dsi_driver(rad_panel_driver);
+
+MODULE_AUTHOR("Robert Chiras <robert.chiras@nxp.com>");
+MODULE_DESCRIPTION("DRM Driver for Raydium RM67191 MIPI DSI panel");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v3 2/2] drm/panel: Add support for Raydium RM67191 panel driver

[Fabio Estevam]

Hi Robert,

On Thu, Jun 20, 2019 at 10:31 AM Robert Chiras <robert.chiras@nxp.com> wrote:

> +fail:
> +       if (rad->reset)
> +               gpiod_set_value_cansleep(rad->reset, 1);

gpiod_set_value_cansleep() can handle NULL, so no need for the if() check.

> +static const struct display_timing rad_default_timing = {
> +       .pixelclock = { 132000000, 132000000, 132000000 },

Having the same information listed three times does not seem useful.

You could use a drm_display_mode structure with a single entry instead.

> +       videomode_from_timing(&rad_default_timing, &panel->vm);
> +
> +       of_property_read_u32(np, "width-mm", &panel->width_mm);
> +       of_property_read_u32(np, "height-mm", &panel->height_mm);
> +
> +       panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);

Since this is optional it would be better to use
devm_gpiod_get_optional() instead.


> +
> +       if (IS_ERR(panel->reset))
> +               panel->reset = NULL;
> +       else
> +               gpiod_set_value_cansleep(panel->reset, 1);

This is not handling defer probing, so it would be better to do like this:

panel->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(panel->reset))
      return  PTR_ERR(panel->reset);

> +       memset(&bl_props, 0, sizeof(bl_props));
> +       bl_props.type = BACKLIGHT_RAW;
> +       bl_props.brightness = 255;
> +       bl_props.max_brightness = 255;
> +
> +       panel->backlight = devm_backlight_device_register(dev, dev_name(dev),
> +                                                         dev, dsi,
> +                                                         &rad_bl_ops,
> +                                                         &bl_props);

Could you put more parameters into the same line?

Using 4 lines seems excessive.

> +       if (IS_ERR(panel->backlight)) {
> +               ret = PTR_ERR(panel->backlight);
> +               dev_err(dev, "Failed to register backlight (%d)\n", ret);
> +               return ret;
> +       }
> +
> +       drm_panel_init(&panel->panel);
> +       panel->panel.funcs = &rad_panel_funcs;
> +       panel->panel.dev = dev;
> +       dev_set_drvdata(dev, panel);
> +
> +       ret = drm_panel_add(&panel->panel);
> +

Unneeded blank line

> +       if (ret < 0)
> +               return ret;
> +
> +       ret = mipi_dsi_attach(dsi);
> +       if (ret < 0)
> +               drm_panel_remove(&panel->panel);
> +
> +       return ret;

You did not handle the "power" regulator.

> +static int __maybe_unused rad_panel_suspend(struct device *dev)
> +{
> +       struct rad_panel *rad = dev_get_drvdata(dev);
> +
> +       if (!rad->reset)
> +               return 0;
> +
> +       devm_gpiod_put(dev, rad->reset);
> +       rad->reset = NULL;
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused rad_panel_resume(struct device *dev)
> +{
> +       struct rad_panel *rad = dev_get_drvdata(dev);
> +
> +       if (rad->reset)
> +               return 0;
> +
> +       rad->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);

Why do you call devm_gpiod_get() once again?

I am having a hard time to understand the need for this suspend/resume hooks.

Can't you simply remove them?

Re: [PATCH v3 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

[Fabio Estevam]

Hi Robert,

On Thu, Jun 20, 2019 at 10:32 AM Robert Chiras <robert.chiras@nxp.com> wrote:
>
> Add dt-bindings documentation for Raydium RM67191 DSI panel.
>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  .../bindings/display/panel/raydium,rm67191.txt     | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
>
> diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
> new file mode 100644
> index 0000000..52af272
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
> @@ -0,0 +1,39 @@
> +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol
> +
> +Required properties:
> +- compatible:          "raydium,rm67191"
> +- reg:                 virtual channel for MIPI-DSI protocol
> +                       must be <0>
> +- dsi-lanes:           number of DSI lanes to be used
> +                       must be <3> or <4>
> +- port:                input port node with endpoint definition as
> +                       defined in Documentation/devicetree/bindings/graph.txt;
> +                       the input port should be connected to a MIPI-DSI device
> +                       driver
> +
> +Optional properties:
> +- reset-gpios:         a GPIO spec for the RST_B GPIO pin
> +- width-mm:            see panel-common.txt
> +- height-mm:           see panel-common.txt
> +- video-mode:          0 - burst-mode
> +                       1 - non-burst with sync event
> +                       2 - non-burst with sync po ulse

No power-supply property?

Re: [EXT] Re: [PATCH v3 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

[Robert Chiras]

Hi Fabio,

On Vi, 2019-06-21 at 11:00 -0300, Fabio Estevam wrote:
> Hi Robert,
> 
> On Thu, Jun 20, 2019 at 10:32 AM Robert Chiras <robert.chiras@nxp.com
> > wrote:
> > 
> > 
> > Add dt-bindings documentation for Raydium RM67191 DSI panel.
> > 
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> > ---
> >  .../bindings/display/panel/raydium,rm67191.txt     | 39
> > ++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t
> > xt
> > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t
> > xt
> > new file mode 100644
> > index 0000000..52af272
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t
> > xt
> > @@ -0,0 +1,39 @@
> > +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol
> > +
> > +Required properties:
> > +- compatible:          "raydium,rm67191"
> > +- reg:                 virtual channel for MIPI-DSI protocol
> > +                       must be <0>
> > +- dsi-lanes:           number of DSI lanes to be used
> > +                       must be <3> or <4>
> > +- port:                input port node with endpoint definition as
> > +                       defined in
> > Documentation/devicetree/bindings/graph.txt;
> > +                       the input port should be connected to a
> > MIPI-DSI device
> > +                       driver
> > +
> > +Optional properties:
> > +- reset-gpios:         a GPIO spec for the RST_B GPIO pin
> > +- width-mm:            see panel-common.txt
> > +- height-mm:           see panel-common.txt
> > +- video-mode:          0 - burst-mode
> > +                       1 - non-burst with sync event
> > +                       2 - non-burst with sync po ulse
> No power-supply property?
From what I've seen in the schematics, the power lines on the DSI port
on all the i.MX8 cores are coming from a PMIC providing power for all
the peripherals. Since I didn't find a way to cut the power on a single
peripheral (like DSI, for example) it doesn't make sense for power-
supply property. For now, at least.

Re: [EXT] Re: [PATCH v3 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

[Fabio Estevam]

Hi Robert,

On Fri, Jun 21, 2019 at 11:16 AM Robert Chiras <robert.chiras@nxp.com> wrote:

> From what I've seen in the schematics, the power lines on the DSI port
> on all the i.MX8 cores are coming from a PMIC providing power for all
> the peripherals. Since I didn't find a way to cut the power on a single
> peripheral (like DSI, for example) it doesn't make sense for power-
> supply property. For now, at least.

This panel driver is not supposed to only work with i.MX8 NXP reference boards.

The dt-bindings should be as accurate as possible from day one, so
describing the power-supply is important.

Please look at the panel datasheet and describe the required power
supplies accordingly.

Thanks

Re: [EXT] Re: [PATCH v3 2/2] drm/panel: Add support for Raydium RM67191 panel driver

[Robert Chiras]

Hi Fabio,

Thanks for your feedback. I will handle them all, but for the pm_ops I
have some comments. See inline.

On Vi, 2019-06-21 at 10:59 -0300, Fabio Estevam wrote:
> Hi Robert,
> 
> On Thu, Jun 20, 2019 at 10:31 AM Robert Chiras <robert.chiras@nxp.com
> > wrote:
> 
> > 
> > +fail:
> > +       if (rad->reset)
> > +               gpiod_set_value_cansleep(rad->reset, 1);
> gpiod_set_value_cansleep() can handle NULL, so no need for the if()
> check.
> 
> > 
> > +static const struct display_timing rad_default_timing = {
> > +       .pixelclock = { 132000000, 132000000, 132000000 },
> Having the same information listed three times does not seem useful.
> 
> You could use a drm_display_mode structure with a single entry
> instead.
> 
> > 
> > +       videomode_from_timing(&rad_default_timing, &panel->vm);
> > +
> > +       of_property_read_u32(np, "width-mm", &panel->width_mm);
> > +       of_property_read_u32(np, "height-mm", &panel->height_mm);
> > +
> > +       panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> Since this is optional it would be better to use
> devm_gpiod_get_optional() instead.
> 
> 
> > 
> > +
> > +       if (IS_ERR(panel->reset))
> > +               panel->reset = NULL;
> > +       else
> > +               gpiod_set_value_cansleep(panel->reset, 1);
> This is not handling defer probing, so it would be better to do like
> this:
> 
> panel->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(panel->reset))
>       return  PTR_ERR(panel->reset);
> 
> > 
> > +       memset(&bl_props, 0, sizeof(bl_props));
> > +       bl_props.type = BACKLIGHT_RAW;
> > +       bl_props.brightness = 255;
> > +       bl_props.max_brightness = 255;
> > +
> > +       panel->backlight = devm_backlight_device_register(dev,
> > dev_name(dev),
> > +                                                         dev, dsi,
> > +                                                         &rad_bl_o
> > ps,
> > +                                                         &bl_props
> > );
> Could you put more parameters into the same line?
> 
> Using 4 lines seems excessive.
> 
> > 
> > +       if (IS_ERR(panel->backlight)) {
> > +               ret = PTR_ERR(panel->backlight);
> > +               dev_err(dev, "Failed to register backlight (%d)\n",
> > ret);
> > +               return ret;
> > +       }
> > +
> > +       drm_panel_init(&panel->panel);
> > +       panel->panel.funcs = &rad_panel_funcs;
> > +       panel->panel.dev = dev;
> > +       dev_set_drvdata(dev, panel);
> > +
> > +       ret = drm_panel_add(&panel->panel);
> > +
> Unneeded blank line
> 
> > 
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = mipi_dsi_attach(dsi);
> > +       if (ret < 0)
> > +               drm_panel_remove(&panel->panel);
> > +
> > +       return ret;
> You did not handle the "power" regulator.
There is no need for a power regulator.
> 
> > 
> > +static int __maybe_unused rad_panel_suspend(struct device *dev)
> > +{
> > +       struct rad_panel *rad = dev_get_drvdata(dev);
> > +
> > +       if (!rad->reset)
> > +               return 0;
> > +
> > +       devm_gpiod_put(dev, rad->reset);
> > +       rad->reset = NULL;
> > +
> > +       return 0;
> > +}
> > +
> > +static int __maybe_unused rad_panel_resume(struct device *dev)
> > +{
> > +       struct rad_panel *rad = dev_get_drvdata(dev);
> > +
> > +       if (rad->reset)
> > +               return 0;
> > +
> > +       rad->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> Why do you call devm_gpiod_get() once again?
> 
> I am having a hard time to understand the need for this
> suspend/resume hooks.
> 
> Can't you simply remove them?
The reset gpio pin is shared between the DSI display controller and
touch controller, both found on the same panel. Since, the touch driver
also acceses this gpio pin, in some cases the display can be put to
sleep, but the touch is still active. This is why, during suspend I
release the gpio resource, and I have to take it back in resume.
Without this release/acquire mechanism during suspend/resume, stress-
tests showed some failures: the resume freezes completly.

Re: [EXT] Re: [PATCH v3 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

[Robert Chiras]

On Vi, 2019-06-21 at 12:46 -0300, Fabio Estevam wrote:
> Caution: EXT Email
> 
> Hi Robert,
> 
> On Fri, Jun 21, 2019 at 11:16 AM Robert Chiras <robert.chiras@nxp.com
> > wrote:
> 
> > 
> > From what I've seen in the schematics, the power lines on the DSI
> > port
> > on all the i.MX8 cores are coming from a PMIC providing power for
> > all
> > the peripherals. Since I didn't find a way to cut the power on a
> > single
> > peripheral (like DSI, for example) it doesn't make sense for power-
> > supply property. For now, at least.
> This panel driver is not supposed to only work with i.MX8 NXP
> reference boards.
> 
> The dt-bindings should be as accurate as possible from day one, so
> describing the power-supply is important.
> 
> Please look at the panel datasheet and describe the required power
> supplies accordingly.
OK, I will add the power regulators as they are described in panel
datasheet. I just won't be able to test them.
> 
> Thanks

Re: [EXT] Re: [PATCH v3 2/2] drm/panel: Add support for Raydium RM67191 panel driver

[Fabio Estevam]

Hi Robert,

On Mon, Jun 24, 2019 at 4:44 AM Robert Chiras <robert.chiras@nxp.com> wrote:

> > You did not handle the "power" regulator.
> There is no need for a power regulator.

I am sure the panel requires power to be applied so that it can work :-)

> > Can't you simply remove them?
> The reset gpio pin is shared between the DSI display controller and

Looking at the imx8mq-evk schematics it is not clear for me what pin
in shared between the OLED display and touch screen.

Could you please clarify which pin you are referring to?

> touch controller, both found on the same panel. Since, the touch driver
> also acceses this gpio pin, in some cases the display can be put to
> sleep, but the touch is still active. This is why, during suspend I
> release the gpio resource, and I have to take it back in resume.
> Without this release/acquire mechanism during suspend/resume, stress-
> tests showed some failures: the resume freezes completly.

Looking at the imx8mq-evk dts from the vendor tree I see that the
touchscreen is not supported in mainline yet.

Maybe there is a better way to solve this, so what if you don't
introduce the suspend/resume hooks for now and then we revisit this
after the touchscreen driver is added?

Re: [EXT] Re: [PATCH v3 2/2] drm/panel: Add support for Raydium RM67191 panel driver

[Robert Chiras]

On Lu, 2019-06-24 at 13:12 -0300, Fabio Estevam wrote:
> Caution: EXT Email
> 
> Hi Robert,
> 
> On Mon, Jun 24, 2019 at 4:44 AM Robert Chiras <robert.chiras@nxp.com>
> wrote:
> 
> > 
> > > 
> > > You did not handle the "power" regulator.
> > There is no need for a power regulator.
> I am sure the panel requires power to be applied so that it can work
> :-)
Yes, of course there are power lines to the panel. I will add them to
the next revision.
> 
> > 
> > > 
> > > Can't you simply remove them?
> > The reset gpio pin is shared between the DSI display controller and
> Looking at the imx8mq-evk schematics it is not clear for me what pin
> in shared between the OLED display and touch screen.
> 
> Could you please clarify which pin you are referring to?
It's about the DSI_EN pin on the DSI LCD IF physical port J1501. This
goes into RST_B_1V8 on the panel port which is used for both display
and touch resets.
> 
> > 
> > touch controller, both found on the same panel. Since, the touch
> > driver
> > also acceses this gpio pin, in some cases the display can be put to
> > sleep, but the touch is still active. This is why, during suspend I
> > release the gpio resource, and I have to take it back in resume.
> > Without this release/acquire mechanism during suspend/resume,
> > stress-
> > tests showed some failures: the resume freezes completly.
> Looking at the imx8mq-evk dts from the vendor tree I see that the
> touchscreen is not supported in mainline yet.
> 
> Maybe there is a better way to solve this, so what if you don't
> introduce the suspend/resume hooks for now and then we revisit this
> after the touchscreen driver is added?
You are right. We can do this too. I will remove it for now.