All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/2] drm/panel: Add Samsung AMS495QA01 Panel
@ 2022-09-20 17:09 ` Chris Morgan
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 17:09 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, robh+dt, daniel, airlied,
	sam, thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add the Samsung AMS495QA01 panel as found on the Anbernic RG503. This
panel uses DSI to receive video signals, but 3-wire SPI to receive
command signals.

Changes since V1:
 - Removed errant reference to backlight in documentation. This is an
   OLED panel.
 - Made elvss regulator optional. In my case its hard wired and not
   controllable.
 - Added "prepared" enum to track panel status to prevent unbalanced
   regulator enable/disable.

Chris Morgan (2):
  dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
  drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel

 .../display/panel/samsung,ams495qa01.yaml     |  46 ++
 drivers/gpu/drm/panel/Kconfig                 |  10 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
 4 files changed, 562 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c

-- 
2.25.1


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

* [PATCH V2 0/2] drm/panel: Add Samsung AMS495QA01 Panel
@ 2022-09-20 17:09 ` Chris Morgan
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 17:09 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, airlied, Chris Morgan,
	robh+dt, thierry.reding, sam

From: Chris Morgan <macromorgan@hotmail.com>

Add the Samsung AMS495QA01 panel as found on the Anbernic RG503. This
panel uses DSI to receive video signals, but 3-wire SPI to receive
command signals.

Changes since V1:
 - Removed errant reference to backlight in documentation. This is an
   OLED panel.
 - Made elvss regulator optional. In my case its hard wired and not
   controllable.
 - Added "prepared" enum to track panel status to prevent unbalanced
   regulator enable/disable.

Chris Morgan (2):
  dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
  drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel

 .../display/panel/samsung,ams495qa01.yaml     |  46 ++
 drivers/gpu/drm/panel/Kconfig                 |  10 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
 4 files changed, 562 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c

-- 
2.25.1


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

* [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
  2022-09-20 17:09 ` Chris Morgan
@ 2022-09-20 17:09   ` Chris Morgan
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 17:09 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, robh+dt, daniel, airlied,
	sam, thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add documentation for the Samsung AMS495QA01 panel.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../display/panel/samsung,ams495qa01.yaml     | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
new file mode 100644
index 000000000000..08358cdad19c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung AMS495QA01 4.95in 960x544 DSI/SPI panel
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,ams495qa01
+  reg: true
+  reset-gpios: true
+  elvdd-supply:
+    description: regulator that supplies voltage to the panel display
+  enable-gpios: true
+  port: true
+  vdd-supply:
+    description: regulator that supplies voltage to panel logic
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "samsung,ams495qa01";
+            reg = <0>;
+            vdd-supply = <&vcc3v3_lcd0_n>;
+        };
+    };
+
+...
-- 
2.25.1


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

* [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
@ 2022-09-20 17:09   ` Chris Morgan
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 17:09 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, airlied, Chris Morgan,
	robh+dt, thierry.reding, sam

From: Chris Morgan <macromorgan@hotmail.com>

Add documentation for the Samsung AMS495QA01 panel.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../display/panel/samsung,ams495qa01.yaml     | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
new file mode 100644
index 000000000000..08358cdad19c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung AMS495QA01 4.95in 960x544 DSI/SPI panel
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,ams495qa01
+  reg: true
+  reset-gpios: true
+  elvdd-supply:
+    description: regulator that supplies voltage to the panel display
+  enable-gpios: true
+  port: true
+  vdd-supply:
+    description: regulator that supplies voltage to panel logic
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "samsung,ams495qa01";
+            reg = <0>;
+            vdd-supply = <&vcc3v3_lcd0_n>;
+        };
+    };
+
+...
-- 
2.25.1


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

* [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
  2022-09-20 17:09 ` Chris Morgan
@ 2022-09-20 17:09   ` Chris Morgan
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 17:09 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, robh+dt, daniel, airlied,
	sam, thierry.reding, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Support Samsung AMS495QA01 panel as found on the Anbernic RG503. Note
This panel receives video signals via DSI, however it receives
commands via 3-wire SPI.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |  10 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
 3 files changed, 516 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a9043eacce97..954b66a2adee 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -444,6 +444,16 @@ config DRM_PANEL_RONBO_RB070D30
 	  Say Y here if you want to enable support for Ronbo Electronics
 	  RB070D30 1024x600 DSI panel.
 
+config DRM_PANEL_SAMSUNG_AMS495QA01
+	tristate "Samsung AMS495QA01 DSI panel"
+	depends on OF && SPI
+	depends on DRM_MIPI_DSI
+	select DRM_MIPI_DBI
+	help
+	  DRM panel driver for the Samsung AMS495QA01 panel. This panel
+	  receives video data via DSI but commands via 3-Wire 9-bit
+	  SPI.
+
 config DRM_PANEL_SAMSUNG_ATNA33XC20
 	tristate "Samsung ATNA33XC20 eDP panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 34e717382dbb..de0b57baf851 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
 obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS495QA01) += panel-samsung-ams495qa01.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
new file mode 100644
index 000000000000..d693ba5f20c9
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
@@ -0,0 +1,505 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Samsung ams495qa01 MIPI-DSI panel driver
+ * Copyright (C) 2022 Chris Morgan
+ */
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+struct ams495qa01 {
+	/** @dev: the container device */
+	struct device *dev;
+	/** @dbi: the DBI bus abstraction handle */
+	struct mipi_dbi dbi;
+	/** @panel: the DRM panel instance for this device */
+	struct drm_panel panel;
+	/** @reset: reset GPIO line */
+	struct gpio_desc *reset;
+	/** @enable: enable GPIO line */
+	struct gpio_desc *enable;
+	/** @reg_vdd: VDD supply regulator for panel logic */
+	struct regulator *reg_vdd;
+	/** @reg_elvdd: ELVDD supply regulator for panel display */
+	struct regulator *reg_elvdd;
+	/** @dsi_dev: DSI child device (panel) */
+	struct mipi_dsi_device *dsi_dev;
+	/** @bl_dev: pseudo-backlight device for oled panel */
+	struct backlight_device *bl_dev;
+	/** @prepared: value tracking panel prepare status */
+	bool prepared;
+};
+
+static const struct drm_display_mode ams495qa01_mode = {
+	.clock = 33500,
+	.hdisplay = 960,
+	.hsync_start = 960 + 10,
+	.hsync_end = 960 + 10 + 2,
+	.htotal = 960 + 10 + 2 + 10,
+	.vdisplay = 544,
+	.vsync_start = 544 + 10,
+	.vsync_end = 544 + 10 + 2,
+	.vtotal = 544 + 10 + 2 + 10,
+	.width_mm = 117,
+	.height_mm = 74,
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+#define NUM_GAMMA_LEVELS	16
+#define GAMMA_TABLE_COUNT	23
+#define MAX_BRIGHTNESS		(NUM_GAMMA_LEVELS - 1)
+
+/* Table of gamma values provided in datasheet */
+static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
+	{0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
+	 0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
+	 0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
+	 0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
+	 0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
+	 0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
+	 0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
+	 0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
+	 0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+};
+
+/*
+ * Table of elvss values provided in datasheet and corresponds to
+ * gamma values.
+ */
+static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
+	0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
+	0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
+};
+
+static inline struct ams495qa01 *to_ams495qa01(struct drm_panel *panel)
+{
+	return container_of(panel, struct ams495qa01, panel);
+}
+
+static int ams495qa01_update_gamma(struct mipi_dbi *dbi, u32 brightness)
+{
+	u32 tmp = brightness;
+
+	if (brightness > MAX_BRIGHTNESS)
+		tmp = MAX_BRIGHTNESS;
+
+	/* Set gamma values */
+	mipi_dbi_command_buf(dbi, 0xf9, ams495qa01_gamma[tmp],
+			     ARRAY_SIZE(ams495qa01_gamma[tmp]));
+	/* Set gamma change */
+	mipi_dbi_command(dbi, 0xf9, 0x00);
+	/* Undocumented command */
+	mipi_dbi_command(dbi, 0x26, 0x00);
+	/* Set ELVSS value */
+	mipi_dbi_command(dbi, 0xb2, ams495qa01_elvss[tmp]);
+
+	return 0;
+}
+
+static int ams495qa01_prepare(struct drm_panel *panel)
+{
+	struct ams495qa01 *db = to_ams495qa01(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+	int ret;
+
+	if (db->prepared)
+		return 0;
+
+	/* Power up */
+	ret = regulator_enable(db->reg_vdd);
+	if (ret) {
+		dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
+		return ret;
+	}
+	if (db->reg_elvdd) {
+		ret = regulator_enable(db->reg_elvdd);
+		if (ret) {
+			dev_err(db->dev,
+				"failed to enable elvdd regulator: %d\n", ret);
+			regulator_disable(db->reg_vdd);
+			return ret;
+		}
+	}
+
+	/* Enable */
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 1);
+
+	msleep(50);
+
+	/* Reset */
+	gpiod_set_value_cansleep(db->reset, 1);
+	usleep_range(1000, 5000);
+	gpiod_set_value_cansleep(db->reset, 0);
+	msleep(20);
+
+	/* Panel Init Sequence */
+
+	/* Password to start command sequence */
+	mipi_dbi_command(dbi, 0xf0, 0x5a, 0x5a);
+	mipi_dbi_command(dbi, 0xf1, 0x5a, 0x5a);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb0, 0x02);
+	mipi_dbi_command(dbi, 0xf3, 0x3b);
+
+	/* Analog Power condition set */
+	mipi_dbi_command(dbi, 0xf4, 0x33, 0x42, 0x00, 0x08);
+	mipi_dbi_command(dbi, 0xf5, 0x00, 0x06, 0x26, 0x35, 0x03);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xf6, 0x02);
+	mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
+			 0x00, 0x00, 0x00, 0x00);
+
+	/* GTCON set */
+	mipi_dbi_command(dbi, 0xf7, 0x20);
+
+	/* TEMP_SWIRE set */
+	mipi_dbi_command(dbi, 0xb2, 0x06, 0x06, 0x06, 0x06);
+
+	/* ELVSS_CON set */
+	mipi_dbi_command(dbi, 0xb1, 0x07, 0x00, 0x10);
+
+	/* Gateless signal set */
+	mipi_dbi_command(dbi, 0xf8, 0x7f, 0x7a, 0x89, 0x67, 0x26, 0x38,
+			 0x00, 0x00, 0x09, 0x67, 0x70, 0x88, 0x7a,
+			 0x76, 0x05, 0x09, 0x23, 0x23, 0x23);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
+			 0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
+			 0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
+			 0xff);
+	mipi_dbi_command(dbi, 0xb4, 0x15);
+	mipi_dbi_command(dbi, 0xb3, 0x00);
+
+	ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
+
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(200);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+	usleep_range(10000, 15000);
+
+	db->prepared = true;
+
+	return 0;
+}
+
+static int ams495qa01_unprepare(struct drm_panel *panel)
+{
+	struct ams495qa01 *db = to_ams495qa01(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	if (!db->prepared)
+		return 0;
+
+	/* Panel Exit Sequence */
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(20);
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
+	usleep_range(10000, 15000);
+
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 0);
+	if (db->reg_elvdd)
+		regulator_disable(db->reg_elvdd);
+	regulator_disable(db->reg_vdd);
+	msleep(20);
+
+	db->prepared = false;
+
+	return 0;
+}
+
+static int ams495qa01_get_modes(struct drm_panel *panel,
+				struct drm_connector *connector)
+{
+	struct ams495qa01 *db = to_ams495qa01(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, &ams495qa01_mode);
+	if (!mode) {
+		dev_err(db->dev, "failed to add mode\n");
+		return -ENOMEM;
+	}
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	connector->display_info.bus_flags =
+		DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
+		DRM_BUS_FLAG_DE_LOW;
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	drm_mode_set_name(mode);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ams495qa01_drm_funcs = {
+	.unprepare = ams495qa01_unprepare,
+	.prepare = ams495qa01_prepare,
+	.get_modes = ams495qa01_get_modes,
+};
+
+static int ams495qa01_set_brightness(struct backlight_device *bd)
+{
+	struct ams495qa01 *db = bl_get_data(bd);
+	struct mipi_dbi *dbi = &db->dbi;
+	int brightness = bd->props.brightness;
+
+	ams495qa01_update_gamma(dbi, brightness);
+
+	return 0;
+}
+
+static const struct backlight_ops ams495qa01_backlight_ops = {
+	.update_status	= ams495qa01_set_brightness,
+};
+
+static int ams495qa01_backlight_register(struct ams495qa01 *db)
+{
+	struct backlight_properties props = {
+		.type		= BACKLIGHT_RAW,
+		.brightness	= MAX_BRIGHTNESS,
+		.max_brightness = MAX_BRIGHTNESS,
+	};
+	struct device *dev = db->dev;
+	int ret = 0;
+
+	db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
+						     &ams495qa01_backlight_ops,
+						     &props);
+	if (IS_ERR(db->bl_dev)) {
+		ret = PTR_ERR(db->bl_dev);
+		dev_err(dev, "error registering backlight device (%d)\n", ret);
+	}
+
+	return ret;
+}
+
+static int ams495qa01_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct device_node *endpoint, *dsi_host_node;
+	struct mipi_dsi_host *dsi_host;
+	struct ams495qa01 *db;
+	int ret;
+	struct mipi_dsi_device_info info = {
+		.type = "dupa",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, db);
+
+	db->dev = dev;
+
+	db->reg_vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(db->reg_vdd))
+		return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
+				     "Failed to get vdd supply\n");
+
+	db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
+	if (IS_ERR(db->reg_elvdd))
+		db->reg_elvdd = NULL;
+
+	db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(db->reset)) {
+		ret = PTR_ERR(db->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(db->enable)) {
+		ret = PTR_ERR(db->enable);
+		return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	/*
+	 * Get the DSI controller that is supplying data for this display
+	 * which is controlled via SPI 3-wire.
+	 */
+	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!endpoint) {
+		dev_err(dev, "failed to get endpoint\n");
+		return -ENODEV;
+	}
+	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+	if (!dsi_host_node) {
+		dev_err(dev, "failed to get remote port parent\n");
+		goto put_endpoint;
+	}
+	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+	if (!dsi_host) {
+		dev_err(dev, "failed to find dsi host\n");
+		goto put_host;
+	}
+	info.node = of_graph_get_remote_port(endpoint);
+	if (!info.node) {
+		dev_err(dev, "failed to get remote port node\n");
+		ret = -ENODEV;
+		goto put_host;
+	}
+
+	db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
+	if (IS_ERR(db->dsi_dev)) {
+		dev_err(dev, "failed to register dsi device: %ld\n",
+			PTR_ERR(db->dsi_dev));
+		ret = PTR_ERR(db->dsi_dev);
+		goto put_host;
+	}
+
+	db->dsi_dev->lanes = 2;
+	db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
+	db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
+
+	drm_panel_init(&db->panel, dev, &ams495qa01_drm_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	ret = ams495qa01_backlight_register(db);
+	if (ret < 0)
+		return ret;
+
+	drm_panel_add(&db->panel);
+
+	ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
+	if (ret < 0) {
+		dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&db->panel);
+		return ret;
+	}
+
+	of_node_put(dsi_host_node);
+	of_node_put(endpoint);
+
+	return 0;
+
+put_host:
+	of_node_put(dsi_host_node);
+
+put_endpoint:
+	of_node_put(endpoint);
+	return -ENODEV;
+}
+
+static void ams495qa01_shutdown(struct spi_device *spi)
+{
+	struct ams495qa01 *db = spi_get_drvdata(spi);
+	int ret;
+
+	ret = drm_panel_unprepare(&db->panel);
+	if (ret < 0)
+		dev_err(&spi->dev, "Failed to unprepare panel: %d\n", ret);
+
+	ret = drm_panel_disable(&db->panel);
+	if (ret < 0)
+		dev_err(&spi->dev, "Failed to disable panel: %d\n", ret);
+}
+
+static void ams495qa01_remove(struct spi_device *spi)
+{
+	struct ams495qa01 *db = spi_get_drvdata(spi);
+
+	ams495qa01_shutdown(spi);
+
+	drm_panel_remove(&db->panel);
+}
+
+static const struct of_device_id ams495qa01_match[] = {
+	{ .compatible = "samsung,ams495qa01", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ams495qa01_match);
+
+static const struct spi_device_id ams495qa01_ids[] = {
+	{ "ams495qa01", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, ams495qa01_ids);
+
+static struct spi_driver ams495qa01_driver = {
+	.driver		= {
+		.name	= "ams495qa01-panel",
+		.of_match_table = ams495qa01_match,
+	},
+	.id_table	= ams495qa01_ids,
+	.probe		= ams495qa01_probe,
+	.remove		= ams495qa01_remove,
+	.shutdown	= ams495qa01_shutdown,
+};
+module_spi_driver(ams495qa01_driver);
+
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_DESCRIPTION("Samsung ams495qa01 panel driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

* [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
@ 2022-09-20 17:09   ` Chris Morgan
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 17:09 UTC (permalink / raw)
  To: dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, airlied, Chris Morgan,
	robh+dt, thierry.reding, sam

From: Chris Morgan <macromorgan@hotmail.com>

Support Samsung AMS495QA01 panel as found on the Anbernic RG503. Note
This panel receives video signals via DSI, however it receives
commands via 3-wire SPI.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |  10 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
 3 files changed, 516 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a9043eacce97..954b66a2adee 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -444,6 +444,16 @@ config DRM_PANEL_RONBO_RB070D30
 	  Say Y here if you want to enable support for Ronbo Electronics
 	  RB070D30 1024x600 DSI panel.
 
+config DRM_PANEL_SAMSUNG_AMS495QA01
+	tristate "Samsung AMS495QA01 DSI panel"
+	depends on OF && SPI
+	depends on DRM_MIPI_DSI
+	select DRM_MIPI_DBI
+	help
+	  DRM panel driver for the Samsung AMS495QA01 panel. This panel
+	  receives video data via DSI but commands via 3-Wire 9-bit
+	  SPI.
+
 config DRM_PANEL_SAMSUNG_ATNA33XC20
 	tristate "Samsung ATNA33XC20 eDP panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 34e717382dbb..de0b57baf851 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
 obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS495QA01) += panel-samsung-ams495qa01.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
new file mode 100644
index 000000000000..d693ba5f20c9
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
@@ -0,0 +1,505 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Samsung ams495qa01 MIPI-DSI panel driver
+ * Copyright (C) 2022 Chris Morgan
+ */
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+struct ams495qa01 {
+	/** @dev: the container device */
+	struct device *dev;
+	/** @dbi: the DBI bus abstraction handle */
+	struct mipi_dbi dbi;
+	/** @panel: the DRM panel instance for this device */
+	struct drm_panel panel;
+	/** @reset: reset GPIO line */
+	struct gpio_desc *reset;
+	/** @enable: enable GPIO line */
+	struct gpio_desc *enable;
+	/** @reg_vdd: VDD supply regulator for panel logic */
+	struct regulator *reg_vdd;
+	/** @reg_elvdd: ELVDD supply regulator for panel display */
+	struct regulator *reg_elvdd;
+	/** @dsi_dev: DSI child device (panel) */
+	struct mipi_dsi_device *dsi_dev;
+	/** @bl_dev: pseudo-backlight device for oled panel */
+	struct backlight_device *bl_dev;
+	/** @prepared: value tracking panel prepare status */
+	bool prepared;
+};
+
+static const struct drm_display_mode ams495qa01_mode = {
+	.clock = 33500,
+	.hdisplay = 960,
+	.hsync_start = 960 + 10,
+	.hsync_end = 960 + 10 + 2,
+	.htotal = 960 + 10 + 2 + 10,
+	.vdisplay = 544,
+	.vsync_start = 544 + 10,
+	.vsync_end = 544 + 10 + 2,
+	.vtotal = 544 + 10 + 2 + 10,
+	.width_mm = 117,
+	.height_mm = 74,
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+#define NUM_GAMMA_LEVELS	16
+#define GAMMA_TABLE_COUNT	23
+#define MAX_BRIGHTNESS		(NUM_GAMMA_LEVELS - 1)
+
+/* Table of gamma values provided in datasheet */
+static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
+	{0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
+	 0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
+	 0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
+	 0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
+	 0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
+	 0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
+	 0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
+	 0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
+	 0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
+	 0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
+	 0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
+	 0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+	{0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
+	 0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
+	 0x00, 0x2f},
+};
+
+/*
+ * Table of elvss values provided in datasheet and corresponds to
+ * gamma values.
+ */
+static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
+	0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
+	0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
+};
+
+static inline struct ams495qa01 *to_ams495qa01(struct drm_panel *panel)
+{
+	return container_of(panel, struct ams495qa01, panel);
+}
+
+static int ams495qa01_update_gamma(struct mipi_dbi *dbi, u32 brightness)
+{
+	u32 tmp = brightness;
+
+	if (brightness > MAX_BRIGHTNESS)
+		tmp = MAX_BRIGHTNESS;
+
+	/* Set gamma values */
+	mipi_dbi_command_buf(dbi, 0xf9, ams495qa01_gamma[tmp],
+			     ARRAY_SIZE(ams495qa01_gamma[tmp]));
+	/* Set gamma change */
+	mipi_dbi_command(dbi, 0xf9, 0x00);
+	/* Undocumented command */
+	mipi_dbi_command(dbi, 0x26, 0x00);
+	/* Set ELVSS value */
+	mipi_dbi_command(dbi, 0xb2, ams495qa01_elvss[tmp]);
+
+	return 0;
+}
+
+static int ams495qa01_prepare(struct drm_panel *panel)
+{
+	struct ams495qa01 *db = to_ams495qa01(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+	int ret;
+
+	if (db->prepared)
+		return 0;
+
+	/* Power up */
+	ret = regulator_enable(db->reg_vdd);
+	if (ret) {
+		dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
+		return ret;
+	}
+	if (db->reg_elvdd) {
+		ret = regulator_enable(db->reg_elvdd);
+		if (ret) {
+			dev_err(db->dev,
+				"failed to enable elvdd regulator: %d\n", ret);
+			regulator_disable(db->reg_vdd);
+			return ret;
+		}
+	}
+
+	/* Enable */
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 1);
+
+	msleep(50);
+
+	/* Reset */
+	gpiod_set_value_cansleep(db->reset, 1);
+	usleep_range(1000, 5000);
+	gpiod_set_value_cansleep(db->reset, 0);
+	msleep(20);
+
+	/* Panel Init Sequence */
+
+	/* Password to start command sequence */
+	mipi_dbi_command(dbi, 0xf0, 0x5a, 0x5a);
+	mipi_dbi_command(dbi, 0xf1, 0x5a, 0x5a);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb0, 0x02);
+	mipi_dbi_command(dbi, 0xf3, 0x3b);
+
+	/* Analog Power condition set */
+	mipi_dbi_command(dbi, 0xf4, 0x33, 0x42, 0x00, 0x08);
+	mipi_dbi_command(dbi, 0xf5, 0x00, 0x06, 0x26, 0x35, 0x03);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xf6, 0x02);
+	mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
+			 0x00, 0x00, 0x00, 0x00);
+
+	/* GTCON set */
+	mipi_dbi_command(dbi, 0xf7, 0x20);
+
+	/* TEMP_SWIRE set */
+	mipi_dbi_command(dbi, 0xb2, 0x06, 0x06, 0x06, 0x06);
+
+	/* ELVSS_CON set */
+	mipi_dbi_command(dbi, 0xb1, 0x07, 0x00, 0x10);
+
+	/* Gateless signal set */
+	mipi_dbi_command(dbi, 0xf8, 0x7f, 0x7a, 0x89, 0x67, 0x26, 0x38,
+			 0x00, 0x00, 0x09, 0x67, 0x70, 0x88, 0x7a,
+			 0x76, 0x05, 0x09, 0x23, 0x23, 0x23);
+
+	/* Undocumented commands */
+	mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
+			 0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
+			 0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
+			 0xff);
+	mipi_dbi_command(dbi, 0xb4, 0x15);
+	mipi_dbi_command(dbi, 0xb3, 0x00);
+
+	ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
+
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(200);
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+	usleep_range(10000, 15000);
+
+	db->prepared = true;
+
+	return 0;
+}
+
+static int ams495qa01_unprepare(struct drm_panel *panel)
+{
+	struct ams495qa01 *db = to_ams495qa01(panel);
+	struct mipi_dbi *dbi = &db->dbi;
+
+	if (!db->prepared)
+		return 0;
+
+	/* Panel Exit Sequence */
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(20);
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
+	usleep_range(10000, 15000);
+
+	if (db->enable)
+		gpiod_set_value_cansleep(db->enable, 0);
+	if (db->reg_elvdd)
+		regulator_disable(db->reg_elvdd);
+	regulator_disable(db->reg_vdd);
+	msleep(20);
+
+	db->prepared = false;
+
+	return 0;
+}
+
+static int ams495qa01_get_modes(struct drm_panel *panel,
+				struct drm_connector *connector)
+{
+	struct ams495qa01 *db = to_ams495qa01(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, &ams495qa01_mode);
+	if (!mode) {
+		dev_err(db->dev, "failed to add mode\n");
+		return -ENOMEM;
+	}
+
+	connector->display_info.bpc = 8;
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	connector->display_info.bus_flags =
+		DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
+		DRM_BUS_FLAG_DE_LOW;
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	drm_mode_set_name(mode);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ams495qa01_drm_funcs = {
+	.unprepare = ams495qa01_unprepare,
+	.prepare = ams495qa01_prepare,
+	.get_modes = ams495qa01_get_modes,
+};
+
+static int ams495qa01_set_brightness(struct backlight_device *bd)
+{
+	struct ams495qa01 *db = bl_get_data(bd);
+	struct mipi_dbi *dbi = &db->dbi;
+	int brightness = bd->props.brightness;
+
+	ams495qa01_update_gamma(dbi, brightness);
+
+	return 0;
+}
+
+static const struct backlight_ops ams495qa01_backlight_ops = {
+	.update_status	= ams495qa01_set_brightness,
+};
+
+static int ams495qa01_backlight_register(struct ams495qa01 *db)
+{
+	struct backlight_properties props = {
+		.type		= BACKLIGHT_RAW,
+		.brightness	= MAX_BRIGHTNESS,
+		.max_brightness = MAX_BRIGHTNESS,
+	};
+	struct device *dev = db->dev;
+	int ret = 0;
+
+	db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
+						     &ams495qa01_backlight_ops,
+						     &props);
+	if (IS_ERR(db->bl_dev)) {
+		ret = PTR_ERR(db->bl_dev);
+		dev_err(dev, "error registering backlight device (%d)\n", ret);
+	}
+
+	return ret;
+}
+
+static int ams495qa01_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct device_node *endpoint, *dsi_host_node;
+	struct mipi_dsi_host *dsi_host;
+	struct ams495qa01 *db;
+	int ret;
+	struct mipi_dsi_device_info info = {
+		.type = "dupa",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, db);
+
+	db->dev = dev;
+
+	db->reg_vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(db->reg_vdd))
+		return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
+				     "Failed to get vdd supply\n");
+
+	db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
+	if (IS_ERR(db->reg_elvdd))
+		db->reg_elvdd = NULL;
+
+	db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(db->reset)) {
+		ret = PTR_ERR(db->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(db->enable)) {
+		ret = PTR_ERR(db->enable);
+		return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	/*
+	 * Get the DSI controller that is supplying data for this display
+	 * which is controlled via SPI 3-wire.
+	 */
+	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!endpoint) {
+		dev_err(dev, "failed to get endpoint\n");
+		return -ENODEV;
+	}
+	dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+	if (!dsi_host_node) {
+		dev_err(dev, "failed to get remote port parent\n");
+		goto put_endpoint;
+	}
+	dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+	if (!dsi_host) {
+		dev_err(dev, "failed to find dsi host\n");
+		goto put_host;
+	}
+	info.node = of_graph_get_remote_port(endpoint);
+	if (!info.node) {
+		dev_err(dev, "failed to get remote port node\n");
+		ret = -ENODEV;
+		goto put_host;
+	}
+
+	db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
+	if (IS_ERR(db->dsi_dev)) {
+		dev_err(dev, "failed to register dsi device: %ld\n",
+			PTR_ERR(db->dsi_dev));
+		ret = PTR_ERR(db->dsi_dev);
+		goto put_host;
+	}
+
+	db->dsi_dev->lanes = 2;
+	db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
+	db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
+
+	drm_panel_init(&db->panel, dev, &ams495qa01_drm_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	ret = ams495qa01_backlight_register(db);
+	if (ret < 0)
+		return ret;
+
+	drm_panel_add(&db->panel);
+
+	ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
+	if (ret < 0) {
+		dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&db->panel);
+		return ret;
+	}
+
+	of_node_put(dsi_host_node);
+	of_node_put(endpoint);
+
+	return 0;
+
+put_host:
+	of_node_put(dsi_host_node);
+
+put_endpoint:
+	of_node_put(endpoint);
+	return -ENODEV;
+}
+
+static void ams495qa01_shutdown(struct spi_device *spi)
+{
+	struct ams495qa01 *db = spi_get_drvdata(spi);
+	int ret;
+
+	ret = drm_panel_unprepare(&db->panel);
+	if (ret < 0)
+		dev_err(&spi->dev, "Failed to unprepare panel: %d\n", ret);
+
+	ret = drm_panel_disable(&db->panel);
+	if (ret < 0)
+		dev_err(&spi->dev, "Failed to disable panel: %d\n", ret);
+}
+
+static void ams495qa01_remove(struct spi_device *spi)
+{
+	struct ams495qa01 *db = spi_get_drvdata(spi);
+
+	ams495qa01_shutdown(spi);
+
+	drm_panel_remove(&db->panel);
+}
+
+static const struct of_device_id ams495qa01_match[] = {
+	{ .compatible = "samsung,ams495qa01", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ams495qa01_match);
+
+static const struct spi_device_id ams495qa01_ids[] = {
+	{ "ams495qa01", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, ams495qa01_ids);
+
+static struct spi_driver ams495qa01_driver = {
+	.driver		= {
+		.name	= "ams495qa01-panel",
+		.of_match_table = ams495qa01_match,
+	},
+	.id_table	= ams495qa01_ids,
+	.probe		= ams495qa01_probe,
+	.remove		= ams495qa01_remove,
+	.shutdown	= ams495qa01_shutdown,
+};
+module_spi_driver(ams495qa01_driver);
+
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_DESCRIPTION("Samsung ams495qa01 panel driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

* Re: [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
  2022-09-20 17:09   ` Chris Morgan
@ 2022-09-20 17:33     ` Maya Matuszczyk
  -1 siblings, 0 replies; 14+ messages in thread
From: Maya Matuszczyk @ 2022-09-20 17:33 UTC (permalink / raw)
  To: Chris Morgan
  Cc: dri-devel, devicetree, krzysztof.kozlowski+dt, airlied,
	Chris Morgan, robh+dt, thierry.reding, sam

Hi Chris,
Thanks for this patch,

wt., 20 wrz 2022 o 19:10 Chris Morgan <macroalpha82@gmail.com> napisał(a):
>
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Support Samsung AMS495QA01 panel as found on the Anbernic RG503. Note
> This panel receives video signals via DSI, however it receives
> commands via 3-wire SPI.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |  10 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
>  3 files changed, 516 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index a9043eacce97..954b66a2adee 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -444,6 +444,16 @@ config DRM_PANEL_RONBO_RB070D30
>           Say Y here if you want to enable support for Ronbo Electronics
>           RB070D30 1024x600 DSI panel.
>
> +config DRM_PANEL_SAMSUNG_AMS495QA01
> +       tristate "Samsung AMS495QA01 DSI panel"
> +       depends on OF && SPI
> +       depends on DRM_MIPI_DSI
> +       select DRM_MIPI_DBI
> +       help
> +         DRM panel driver for the Samsung AMS495QA01 panel. This panel
> +         receives video data via DSI but commands via 3-Wire 9-bit
> +         SPI.
> +
>  config DRM_PANEL_SAMSUNG_ATNA33XC20
>         tristate "Samsung ATNA33XC20 eDP panel"
>         depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 34e717382dbb..de0b57baf851 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen
>  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
>  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
>  obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS495QA01) += panel-samsung-ams495qa01.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> new file mode 100644
> index 000000000000..d693ba5f20c9
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> @@ -0,0 +1,505 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung ams495qa01 MIPI-DSI panel driver
> + * Copyright (C) 2022 Chris Morgan
> + */
> +
> +#include <drm/drm_mipi_dbi.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ams495qa01 {
> +       /** @dev: the container device */
> +       struct device *dev;
> +       /** @dbi: the DBI bus abstraction handle */
> +       struct mipi_dbi dbi;
> +       /** @panel: the DRM panel instance for this device */
> +       struct drm_panel panel;
> +       /** @reset: reset GPIO line */
> +       struct gpio_desc *reset;
> +       /** @enable: enable GPIO line */
> +       struct gpio_desc *enable;
> +       /** @reg_vdd: VDD supply regulator for panel logic */
> +       struct regulator *reg_vdd;
> +       /** @reg_elvdd: ELVDD supply regulator for panel display */
> +       struct regulator *reg_elvdd;
> +       /** @dsi_dev: DSI child device (panel) */
> +       struct mipi_dsi_device *dsi_dev;
> +       /** @bl_dev: pseudo-backlight device for oled panel */
> +       struct backlight_device *bl_dev;
> +       /** @prepared: value tracking panel prepare status */
> +       bool prepared;
> +};
> +
> +static const struct drm_display_mode ams495qa01_mode = {
> +       .clock = 33500,
> +       .hdisplay = 960,
> +       .hsync_start = 960 + 10,
> +       .hsync_end = 960 + 10 + 2,
> +       .htotal = 960 + 10 + 2 + 10,
> +       .vdisplay = 544,
> +       .vsync_start = 544 + 10,
> +       .vsync_end = 544 + 10 + 2,
> +       .vtotal = 544 + 10 + 2 + 10,
> +       .width_mm = 117,
> +       .height_mm = 74,
> +       .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +#define NUM_GAMMA_LEVELS       16
> +#define GAMMA_TABLE_COUNT      23
> +#define MAX_BRIGHTNESS         (NUM_GAMMA_LEVELS - 1)
> +
> +/* Table of gamma values provided in datasheet */
> +static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
> +       {0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
> +        0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
> +        0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
> +        0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
> +        0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
> +        0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
> +        0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
> +        0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
> +        0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
> +        0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
> +        0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
> +        0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
> +        0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
> +        0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
> +        0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
> +        0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
> +        0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
There might be some use in adding unofficial gamma values,
for higher and lower brightness values than officially supported,
For example, vitabright repository of devnoname120 contains
gamma lookup tables that should be compatible with this
display.
Perhaps it's worth checking out, for low-light and outdoors
use case, as the display is meant to be used in handheld
gaming devices:
https://github.com/devnoname120/vitabright

> +};
> +
> +/*
> + * Table of elvss values provided in datasheet and corresponds to
> + * gamma values.
> + */
> +static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
> +       0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
> +       0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
> +};
> +
> +static inline struct ams495qa01 *to_ams495qa01(struct drm_panel *panel)
> +{
> +       return container_of(panel, struct ams495qa01, panel);
> +}
> +
> +static int ams495qa01_update_gamma(struct mipi_dbi *dbi, u32 brightness)
> +{
> +       u32 tmp = brightness;
> +
> +       if (brightness > MAX_BRIGHTNESS)
> +               tmp = MAX_BRIGHTNESS;
Shouldn't we return -EINVAL in this case?

> +
> +       /* Set gamma values */
> +       mipi_dbi_command_buf(dbi, 0xf9, ams495qa01_gamma[tmp],
> +                            ARRAY_SIZE(ams495qa01_gamma[tmp]));
> +       /* Set gamma change */
> +       mipi_dbi_command(dbi, 0xf9, 0x00);
> +       /* Undocumented command */
> +       mipi_dbi_command(dbi, 0x26, 0x00);
> +       /* Set ELVSS value */
> +       mipi_dbi_command(dbi, 0xb2, ams495qa01_elvss[tmp]);
> +
> +       return 0;
> +}
> +
> +static int ams495qa01_prepare(struct drm_panel *panel)
> +{
> +       struct ams495qa01 *db = to_ams495qa01(panel);
> +       struct mipi_dbi *dbi = &db->dbi;
> +       int ret;
> +
> +       if (db->prepared)
> +               return 0;
> +
> +       /* Power up */
> +       ret = regulator_enable(db->reg_vdd);
> +       if (ret) {
> +               dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
> +               return ret;
> +       }
> +       if (db->reg_elvdd) {
> +               ret = regulator_enable(db->reg_elvdd);
> +               if (ret) {
> +                       dev_err(db->dev,
> +                               "failed to enable elvdd regulator: %d\n", ret);
> +                       regulator_disable(db->reg_vdd);
> +                       return ret;
> +               }
> +       }
Maybe ret could be initialized with 0 value, and couple gotos like
with some other drivers?

> +
> +       /* Enable */
> +       if (db->enable)
> +               gpiod_set_value_cansleep(db->enable, 1);
> +
> +       msleep(50);
> +
> +       /* Reset */
> +       gpiod_set_value_cansleep(db->reset, 1);
> +       usleep_range(1000, 5000);
> +       gpiod_set_value_cansleep(db->reset, 0);
> +       msleep(20);
> +
> +       /* Panel Init Sequence */
> +
> +       /* Password to start command sequence */
> +       mipi_dbi_command(dbi, 0xf0, 0x5a, 0x5a);
> +       mipi_dbi_command(dbi, 0xf1, 0x5a, 0x5a);
> +
> +       /* Undocumented commands */
> +       mipi_dbi_command(dbi, 0xb0, 0x02);
> +       mipi_dbi_command(dbi, 0xf3, 0x3b);
> +
> +       /* Analog Power condition set */
> +       mipi_dbi_command(dbi, 0xf4, 0x33, 0x42, 0x00, 0x08);
> +       mipi_dbi_command(dbi, 0xf5, 0x00, 0x06, 0x26, 0x35, 0x03);
> +
> +       /* Undocumented commands */
> +       mipi_dbi_command(dbi, 0xf6, 0x02);
> +       mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
> +                        0x00, 0x00, 0x00, 0x00);
> +
> +       /* GTCON set */
> +       mipi_dbi_command(dbi, 0xf7, 0x20);
> +
> +       /* TEMP_SWIRE set */
> +       mipi_dbi_command(dbi, 0xb2, 0x06, 0x06, 0x06, 0x06);
> +
> +       /* ELVSS_CON set */
> +       mipi_dbi_command(dbi, 0xb1, 0x07, 0x00, 0x10);
> +
> +       /* Gateless signal set */
> +       mipi_dbi_command(dbi, 0xf8, 0x7f, 0x7a, 0x89, 0x67, 0x26, 0x38,
> +                        0x00, 0x00, 0x09, 0x67, 0x70, 0x88, 0x7a,
> +                        0x76, 0x05, 0x09, 0x23, 0x23, 0x23);
> +
> +       /* Undocumented commands */
> +       mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
> +                        0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
> +                        0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
> +                        0xff);
> +       mipi_dbi_command(dbi, 0xb4, 0x15);
> +       mipi_dbi_command(dbi, 0xb3, 0x00);
> +
> +       ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
> +
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       msleep(200);
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
> +       usleep_range(10000, 15000);
> +
> +       db->prepared = true;
> +
> +       return 0;
> +}
> +
> +static int ams495qa01_unprepare(struct drm_panel *panel)
> +{
> +       struct ams495qa01 *db = to_ams495qa01(panel);
> +       struct mipi_dbi *dbi = &db->dbi;
> +
> +       if (!db->prepared)
> +               return 0;
> +
> +       /* Panel Exit Sequence */
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> +       msleep(20);
> +       mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
> +       usleep_range(10000, 15000);
> +
> +       if (db->enable)
> +               gpiod_set_value_cansleep(db->enable, 0);
> +       if (db->reg_elvdd)
> +               regulator_disable(db->reg_elvdd);
> +       regulator_disable(db->reg_vdd);
> +       msleep(20);
> +
> +       db->prepared = false;
> +
> +       return 0;
> +}
> +
> +static int ams495qa01_get_modes(struct drm_panel *panel,
> +                               struct drm_connector *connector)
> +{
> +       struct ams495qa01 *db = to_ams495qa01(panel);
> +       struct drm_display_mode *mode;
> +       static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +
> +       mode = drm_mode_duplicate(connector->dev, &ams495qa01_mode);
> +       if (!mode) {
> +               dev_err(db->dev, "failed to add mode\n");
> +               return -ENOMEM;
> +       }
> +
> +       connector->display_info.bpc = 8;
> +       connector->display_info.width_mm = mode->width_mm;
> +       connector->display_info.height_mm = mode->height_mm;
> +       connector->display_info.bus_flags =
> +               DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
> +               DRM_BUS_FLAG_DE_LOW;
> +       drm_display_info_set_bus_formats(&connector->display_info,
> +                                        &bus_format, 1);
> +
> +       drm_mode_set_name(mode);
> +       mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +
> +       drm_mode_probed_add(connector, mode);
> +
> +       return 1;
> +}
> +
> +static const struct drm_panel_funcs ams495qa01_drm_funcs = {
> +       .unprepare = ams495qa01_unprepare,
> +       .prepare = ams495qa01_prepare,
> +       .get_modes = ams495qa01_get_modes,
> +};
> +
> +static int ams495qa01_set_brightness(struct backlight_device *bd)
> +{
> +       struct ams495qa01 *db = bl_get_data(bd);
> +       struct mipi_dbi *dbi = &db->dbi;
> +       int brightness = bd->props.brightness;
> +
> +       ams495qa01_update_gamma(dbi, brightness);
> +
> +       return 0;
> +}
> +
> +static const struct backlight_ops ams495qa01_backlight_ops = {
> +       .update_status  = ams495qa01_set_brightness,
> +};
> +
> +static int ams495qa01_backlight_register(struct ams495qa01 *db)
> +{
> +       struct backlight_properties props = {
> +               .type           = BACKLIGHT_RAW,
> +               .brightness     = MAX_BRIGHTNESS,
> +               .max_brightness = MAX_BRIGHTNESS,
> +       };
> +       struct device *dev = db->dev;
> +       int ret = 0;
> +
> +       db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
> +                                                    &ams495qa01_backlight_ops,
> +                                                    &props);
> +       if (IS_ERR(db->bl_dev)) {
> +               ret = PTR_ERR(db->bl_dev);
> +               dev_err(dev, "error registering backlight device (%d)\n", ret);
> +       }
> +
> +       return ret;
> +}
> +
> +static int ams495qa01_probe(struct spi_device *spi)
> +{
> +       struct device *dev = &spi->dev;
> +       struct device_node *endpoint, *dsi_host_node;
> +       struct mipi_dsi_host *dsi_host;
> +       struct ams495qa01 *db;
> +       int ret;
> +       struct mipi_dsi_device_info info = {
> +               .type = "dupa",
You might want to change this to something more appropiate for Linux.

I see this was left from a prototype driver that I wrote, which you might
have based this one on.

> +               .channel = 0,
> +               .node = NULL,
> +       };
> +
> +       db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
> +       if (!db)
> +               return -ENOMEM;
> +
> +       spi_set_drvdata(spi, db);
> +
> +       db->dev = dev;
> +
> +       db->reg_vdd = devm_regulator_get(dev, "vdd");
> +       if (IS_ERR(db->reg_vdd))
> +               return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
> +                                    "Failed to get vdd supply\n");
> +
> +       db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
> +       if (IS_ERR(db->reg_elvdd))
> +               db->reg_elvdd = NULL;
> +
> +       db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +       if (IS_ERR(db->reset)) {
> +               ret = PTR_ERR(db->reset);
> +               return dev_err_probe(dev, ret, "no RESET GPIO\n");
I think this can be shortened into:
return dev_err_probe(dev, PTR_ERR(db->reset), "cannot get reset gpio\n");

> +       }
> +
> +       db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> +       if (IS_ERR(db->enable)) {
> +               ret = PTR_ERR(db->enable);
> +               return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
> +       }
> +
> +       ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
It's not always controlled via SPI, sometimes(based on a strap pin)
it's controlled via MIPI-DSI commands.

Maybe let's either have another DT compatible for that?
I'm really not sure

> +       if (ret)
> +               return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> +
> +       /*
> +        * Get the DSI controller that is supplying data for this display
> +        * which is controlled via SPI 3-wire.
Same as above

> +        */
> +       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> +       if (!endpoint) {
> +               dev_err(dev, "failed to get endpoint\n");
> +               return -ENODEV;
> +       }
> +       dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> +       if (!dsi_host_node) {
> +               dev_err(dev, "failed to get remote port parent\n");
> +               goto put_endpoint;
> +       }
> +       dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> +       if (!dsi_host) {
> +               dev_err(dev, "failed to find dsi host\n");
> +               goto put_host;
> +       }
> +       info.node = of_graph_get_remote_port(endpoint);
> +       if (!info.node) {
> +               dev_err(dev, "failed to get remote port node\n");
> +               ret = -ENODEV;
> +               goto put_host;
> +       }
> +
> +       db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
> +       if (IS_ERR(db->dsi_dev)) {
> +               dev_err(dev, "failed to register dsi device: %ld\n",
> +                       PTR_ERR(db->dsi_dev));
> +               ret = PTR_ERR(db->dsi_dev);
> +               goto put_host;
> +       }
> +
> +       db->dsi_dev->lanes = 2;
> +       db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
> +       db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +                         MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> +
> +       drm_panel_init(&db->panel, dev, &ams495qa01_drm_funcs,
> +                      DRM_MODE_CONNECTOR_DSI);
> +
> +       ret = ams495qa01_backlight_register(db);
> +       if (ret < 0)
> +               return ret;
> +
> +       drm_panel_add(&db->panel);
> +
> +       ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
> +       if (ret < 0) {
> +               dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
> +               drm_panel_remove(&db->panel);
> +               return ret;
> +       }
> +
> +       of_node_put(dsi_host_node);
> +       of_node_put(endpoint);
> +
> +       return 0;
> +
> +put_host:
> +       of_node_put(dsi_host_node);
> +
> +put_endpoint:
> +       of_node_put(endpoint);
> +       return -ENODEV;
> +}
> +
> +static void ams495qa01_shutdown(struct spi_device *spi)
> +{
> +       struct ams495qa01 *db = spi_get_drvdata(spi);
> +       int ret;
> +
> +       ret = drm_panel_unprepare(&db->panel);
> +       if (ret < 0)
> +               dev_err(&spi->dev, "Failed to unprepare panel: %d\n", ret);
> +
> +       ret = drm_panel_disable(&db->panel);
> +       if (ret < 0)
> +               dev_err(&spi->dev, "Failed to disable panel: %d\n", ret);
> +}
> +
> +static void ams495qa01_remove(struct spi_device *spi)
> +{
> +       struct ams495qa01 *db = spi_get_drvdata(spi);
> +
> +       ams495qa01_shutdown(spi);
> +
> +       drm_panel_remove(&db->panel);
> +}
> +
> +static const struct of_device_id ams495qa01_match[] = {
> +       { .compatible = "samsung,ams495qa01", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, ams495qa01_match);
> +
> +static const struct spi_device_id ams495qa01_ids[] = {
> +       { "ams495qa01", 0 },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(spi, ams495qa01_ids);
> +
> +static struct spi_driver ams495qa01_driver = {
> +       .driver         = {
> +               .name   = "ams495qa01-panel",
> +               .of_match_table = ams495qa01_match,
> +       },
> +       .id_table       = ams495qa01_ids,
> +       .probe          = ams495qa01_probe,
> +       .remove         = ams495qa01_remove,
> +       .shutdown       = ams495qa01_shutdown,
> +};
> +module_spi_driver(ams495qa01_driver);
> +
> +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> +MODULE_DESCRIPTION("Samsung ams495qa01 panel driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>

Best Regards,
Maya Matuszczyk

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

* Re: [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
@ 2022-09-20 17:33     ` Maya Matuszczyk
  0 siblings, 0 replies; 14+ messages in thread
From: Maya Matuszczyk @ 2022-09-20 17:33 UTC (permalink / raw)
  To: Chris Morgan
  Cc: devicetree, krzysztof.kozlowski+dt, airlied, dri-devel, robh+dt,
	thierry.reding, Chris Morgan, sam

Hi Chris,
Thanks for this patch,

wt., 20 wrz 2022 o 19:10 Chris Morgan <macroalpha82@gmail.com> napisał(a):
>
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Support Samsung AMS495QA01 panel as found on the Anbernic RG503. Note
> This panel receives video signals via DSI, however it receives
> commands via 3-wire SPI.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |  10 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
>  3 files changed, 516 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index a9043eacce97..954b66a2adee 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -444,6 +444,16 @@ config DRM_PANEL_RONBO_RB070D30
>           Say Y here if you want to enable support for Ronbo Electronics
>           RB070D30 1024x600 DSI panel.
>
> +config DRM_PANEL_SAMSUNG_AMS495QA01
> +       tristate "Samsung AMS495QA01 DSI panel"
> +       depends on OF && SPI
> +       depends on DRM_MIPI_DSI
> +       select DRM_MIPI_DBI
> +       help
> +         DRM panel driver for the Samsung AMS495QA01 panel. This panel
> +         receives video data via DSI but commands via 3-Wire 9-bit
> +         SPI.
> +
>  config DRM_PANEL_SAMSUNG_ATNA33XC20
>         tristate "Samsung ATNA33XC20 eDP panel"
>         depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 34e717382dbb..de0b57baf851 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen
>  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
>  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
>  obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS495QA01) += panel-samsung-ams495qa01.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> new file mode 100644
> index 000000000000..d693ba5f20c9
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> @@ -0,0 +1,505 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung ams495qa01 MIPI-DSI panel driver
> + * Copyright (C) 2022 Chris Morgan
> + */
> +
> +#include <drm/drm_mipi_dbi.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ams495qa01 {
> +       /** @dev: the container device */
> +       struct device *dev;
> +       /** @dbi: the DBI bus abstraction handle */
> +       struct mipi_dbi dbi;
> +       /** @panel: the DRM panel instance for this device */
> +       struct drm_panel panel;
> +       /** @reset: reset GPIO line */
> +       struct gpio_desc *reset;
> +       /** @enable: enable GPIO line */
> +       struct gpio_desc *enable;
> +       /** @reg_vdd: VDD supply regulator for panel logic */
> +       struct regulator *reg_vdd;
> +       /** @reg_elvdd: ELVDD supply regulator for panel display */
> +       struct regulator *reg_elvdd;
> +       /** @dsi_dev: DSI child device (panel) */
> +       struct mipi_dsi_device *dsi_dev;
> +       /** @bl_dev: pseudo-backlight device for oled panel */
> +       struct backlight_device *bl_dev;
> +       /** @prepared: value tracking panel prepare status */
> +       bool prepared;
> +};
> +
> +static const struct drm_display_mode ams495qa01_mode = {
> +       .clock = 33500,
> +       .hdisplay = 960,
> +       .hsync_start = 960 + 10,
> +       .hsync_end = 960 + 10 + 2,
> +       .htotal = 960 + 10 + 2 + 10,
> +       .vdisplay = 544,
> +       .vsync_start = 544 + 10,
> +       .vsync_end = 544 + 10 + 2,
> +       .vtotal = 544 + 10 + 2 + 10,
> +       .width_mm = 117,
> +       .height_mm = 74,
> +       .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +#define NUM_GAMMA_LEVELS       16
> +#define GAMMA_TABLE_COUNT      23
> +#define MAX_BRIGHTNESS         (NUM_GAMMA_LEVELS - 1)
> +
> +/* Table of gamma values provided in datasheet */
> +static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
> +       {0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
> +        0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
> +        0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
> +        0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
> +        0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
> +        0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
> +        0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
> +        0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
> +        0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
> +        0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
> +        0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
> +        0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
> +        0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
> +        0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
> +        0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
> +        0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
> +       {0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
> +        0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> +        0x00, 0x2f},
There might be some use in adding unofficial gamma values,
for higher and lower brightness values than officially supported,
For example, vitabright repository of devnoname120 contains
gamma lookup tables that should be compatible with this
display.
Perhaps it's worth checking out, for low-light and outdoors
use case, as the display is meant to be used in handheld
gaming devices:
https://github.com/devnoname120/vitabright

> +};
> +
> +/*
> + * Table of elvss values provided in datasheet and corresponds to
> + * gamma values.
> + */
> +static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
> +       0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
> +       0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
> +};
> +
> +static inline struct ams495qa01 *to_ams495qa01(struct drm_panel *panel)
> +{
> +       return container_of(panel, struct ams495qa01, panel);
> +}
> +
> +static int ams495qa01_update_gamma(struct mipi_dbi *dbi, u32 brightness)
> +{
> +       u32 tmp = brightness;
> +
> +       if (brightness > MAX_BRIGHTNESS)
> +               tmp = MAX_BRIGHTNESS;
Shouldn't we return -EINVAL in this case?

> +
> +       /* Set gamma values */
> +       mipi_dbi_command_buf(dbi, 0xf9, ams495qa01_gamma[tmp],
> +                            ARRAY_SIZE(ams495qa01_gamma[tmp]));
> +       /* Set gamma change */
> +       mipi_dbi_command(dbi, 0xf9, 0x00);
> +       /* Undocumented command */
> +       mipi_dbi_command(dbi, 0x26, 0x00);
> +       /* Set ELVSS value */
> +       mipi_dbi_command(dbi, 0xb2, ams495qa01_elvss[tmp]);
> +
> +       return 0;
> +}
> +
> +static int ams495qa01_prepare(struct drm_panel *panel)
> +{
> +       struct ams495qa01 *db = to_ams495qa01(panel);
> +       struct mipi_dbi *dbi = &db->dbi;
> +       int ret;
> +
> +       if (db->prepared)
> +               return 0;
> +
> +       /* Power up */
> +       ret = regulator_enable(db->reg_vdd);
> +       if (ret) {
> +               dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
> +               return ret;
> +       }
> +       if (db->reg_elvdd) {
> +               ret = regulator_enable(db->reg_elvdd);
> +               if (ret) {
> +                       dev_err(db->dev,
> +                               "failed to enable elvdd regulator: %d\n", ret);
> +                       regulator_disable(db->reg_vdd);
> +                       return ret;
> +               }
> +       }
Maybe ret could be initialized with 0 value, and couple gotos like
with some other drivers?

> +
> +       /* Enable */
> +       if (db->enable)
> +               gpiod_set_value_cansleep(db->enable, 1);
> +
> +       msleep(50);
> +
> +       /* Reset */
> +       gpiod_set_value_cansleep(db->reset, 1);
> +       usleep_range(1000, 5000);
> +       gpiod_set_value_cansleep(db->reset, 0);
> +       msleep(20);
> +
> +       /* Panel Init Sequence */
> +
> +       /* Password to start command sequence */
> +       mipi_dbi_command(dbi, 0xf0, 0x5a, 0x5a);
> +       mipi_dbi_command(dbi, 0xf1, 0x5a, 0x5a);
> +
> +       /* Undocumented commands */
> +       mipi_dbi_command(dbi, 0xb0, 0x02);
> +       mipi_dbi_command(dbi, 0xf3, 0x3b);
> +
> +       /* Analog Power condition set */
> +       mipi_dbi_command(dbi, 0xf4, 0x33, 0x42, 0x00, 0x08);
> +       mipi_dbi_command(dbi, 0xf5, 0x00, 0x06, 0x26, 0x35, 0x03);
> +
> +       /* Undocumented commands */
> +       mipi_dbi_command(dbi, 0xf6, 0x02);
> +       mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
> +                        0x00, 0x00, 0x00, 0x00);
> +
> +       /* GTCON set */
> +       mipi_dbi_command(dbi, 0xf7, 0x20);
> +
> +       /* TEMP_SWIRE set */
> +       mipi_dbi_command(dbi, 0xb2, 0x06, 0x06, 0x06, 0x06);
> +
> +       /* ELVSS_CON set */
> +       mipi_dbi_command(dbi, 0xb1, 0x07, 0x00, 0x10);
> +
> +       /* Gateless signal set */
> +       mipi_dbi_command(dbi, 0xf8, 0x7f, 0x7a, 0x89, 0x67, 0x26, 0x38,
> +                        0x00, 0x00, 0x09, 0x67, 0x70, 0x88, 0x7a,
> +                        0x76, 0x05, 0x09, 0x23, 0x23, 0x23);
> +
> +       /* Undocumented commands */
> +       mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
> +                        0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
> +                        0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
> +                        0xff);
> +       mipi_dbi_command(dbi, 0xb4, 0x15);
> +       mipi_dbi_command(dbi, 0xb3, 0x00);
> +
> +       ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
> +
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       msleep(200);
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
> +       usleep_range(10000, 15000);
> +
> +       db->prepared = true;
> +
> +       return 0;
> +}
> +
> +static int ams495qa01_unprepare(struct drm_panel *panel)
> +{
> +       struct ams495qa01 *db = to_ams495qa01(panel);
> +       struct mipi_dbi *dbi = &db->dbi;
> +
> +       if (!db->prepared)
> +               return 0;
> +
> +       /* Panel Exit Sequence */
> +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> +       msleep(20);
> +       mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
> +       usleep_range(10000, 15000);
> +
> +       if (db->enable)
> +               gpiod_set_value_cansleep(db->enable, 0);
> +       if (db->reg_elvdd)
> +               regulator_disable(db->reg_elvdd);
> +       regulator_disable(db->reg_vdd);
> +       msleep(20);
> +
> +       db->prepared = false;
> +
> +       return 0;
> +}
> +
> +static int ams495qa01_get_modes(struct drm_panel *panel,
> +                               struct drm_connector *connector)
> +{
> +       struct ams495qa01 *db = to_ams495qa01(panel);
> +       struct drm_display_mode *mode;
> +       static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +
> +       mode = drm_mode_duplicate(connector->dev, &ams495qa01_mode);
> +       if (!mode) {
> +               dev_err(db->dev, "failed to add mode\n");
> +               return -ENOMEM;
> +       }
> +
> +       connector->display_info.bpc = 8;
> +       connector->display_info.width_mm = mode->width_mm;
> +       connector->display_info.height_mm = mode->height_mm;
> +       connector->display_info.bus_flags =
> +               DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
> +               DRM_BUS_FLAG_DE_LOW;
> +       drm_display_info_set_bus_formats(&connector->display_info,
> +                                        &bus_format, 1);
> +
> +       drm_mode_set_name(mode);
> +       mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +
> +       drm_mode_probed_add(connector, mode);
> +
> +       return 1;
> +}
> +
> +static const struct drm_panel_funcs ams495qa01_drm_funcs = {
> +       .unprepare = ams495qa01_unprepare,
> +       .prepare = ams495qa01_prepare,
> +       .get_modes = ams495qa01_get_modes,
> +};
> +
> +static int ams495qa01_set_brightness(struct backlight_device *bd)
> +{
> +       struct ams495qa01 *db = bl_get_data(bd);
> +       struct mipi_dbi *dbi = &db->dbi;
> +       int brightness = bd->props.brightness;
> +
> +       ams495qa01_update_gamma(dbi, brightness);
> +
> +       return 0;
> +}
> +
> +static const struct backlight_ops ams495qa01_backlight_ops = {
> +       .update_status  = ams495qa01_set_brightness,
> +};
> +
> +static int ams495qa01_backlight_register(struct ams495qa01 *db)
> +{
> +       struct backlight_properties props = {
> +               .type           = BACKLIGHT_RAW,
> +               .brightness     = MAX_BRIGHTNESS,
> +               .max_brightness = MAX_BRIGHTNESS,
> +       };
> +       struct device *dev = db->dev;
> +       int ret = 0;
> +
> +       db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
> +                                                    &ams495qa01_backlight_ops,
> +                                                    &props);
> +       if (IS_ERR(db->bl_dev)) {
> +               ret = PTR_ERR(db->bl_dev);
> +               dev_err(dev, "error registering backlight device (%d)\n", ret);
> +       }
> +
> +       return ret;
> +}
> +
> +static int ams495qa01_probe(struct spi_device *spi)
> +{
> +       struct device *dev = &spi->dev;
> +       struct device_node *endpoint, *dsi_host_node;
> +       struct mipi_dsi_host *dsi_host;
> +       struct ams495qa01 *db;
> +       int ret;
> +       struct mipi_dsi_device_info info = {
> +               .type = "dupa",
You might want to change this to something more appropiate for Linux.

I see this was left from a prototype driver that I wrote, which you might
have based this one on.

> +               .channel = 0,
> +               .node = NULL,
> +       };
> +
> +       db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
> +       if (!db)
> +               return -ENOMEM;
> +
> +       spi_set_drvdata(spi, db);
> +
> +       db->dev = dev;
> +
> +       db->reg_vdd = devm_regulator_get(dev, "vdd");
> +       if (IS_ERR(db->reg_vdd))
> +               return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
> +                                    "Failed to get vdd supply\n");
> +
> +       db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
> +       if (IS_ERR(db->reg_elvdd))
> +               db->reg_elvdd = NULL;
> +
> +       db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +       if (IS_ERR(db->reset)) {
> +               ret = PTR_ERR(db->reset);
> +               return dev_err_probe(dev, ret, "no RESET GPIO\n");
I think this can be shortened into:
return dev_err_probe(dev, PTR_ERR(db->reset), "cannot get reset gpio\n");

> +       }
> +
> +       db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> +       if (IS_ERR(db->enable)) {
> +               ret = PTR_ERR(db->enable);
> +               return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
> +       }
> +
> +       ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
It's not always controlled via SPI, sometimes(based on a strap pin)
it's controlled via MIPI-DSI commands.

Maybe let's either have another DT compatible for that?
I'm really not sure

> +       if (ret)
> +               return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> +
> +       /*
> +        * Get the DSI controller that is supplying data for this display
> +        * which is controlled via SPI 3-wire.
Same as above

> +        */
> +       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> +       if (!endpoint) {
> +               dev_err(dev, "failed to get endpoint\n");
> +               return -ENODEV;
> +       }
> +       dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> +       if (!dsi_host_node) {
> +               dev_err(dev, "failed to get remote port parent\n");
> +               goto put_endpoint;
> +       }
> +       dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> +       if (!dsi_host) {
> +               dev_err(dev, "failed to find dsi host\n");
> +               goto put_host;
> +       }
> +       info.node = of_graph_get_remote_port(endpoint);
> +       if (!info.node) {
> +               dev_err(dev, "failed to get remote port node\n");
> +               ret = -ENODEV;
> +               goto put_host;
> +       }
> +
> +       db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
> +       if (IS_ERR(db->dsi_dev)) {
> +               dev_err(dev, "failed to register dsi device: %ld\n",
> +                       PTR_ERR(db->dsi_dev));
> +               ret = PTR_ERR(db->dsi_dev);
> +               goto put_host;
> +       }
> +
> +       db->dsi_dev->lanes = 2;
> +       db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
> +       db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +                         MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> +
> +       drm_panel_init(&db->panel, dev, &ams495qa01_drm_funcs,
> +                      DRM_MODE_CONNECTOR_DSI);
> +
> +       ret = ams495qa01_backlight_register(db);
> +       if (ret < 0)
> +               return ret;
> +
> +       drm_panel_add(&db->panel);
> +
> +       ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
> +       if (ret < 0) {
> +               dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
> +               drm_panel_remove(&db->panel);
> +               return ret;
> +       }
> +
> +       of_node_put(dsi_host_node);
> +       of_node_put(endpoint);
> +
> +       return 0;
> +
> +put_host:
> +       of_node_put(dsi_host_node);
> +
> +put_endpoint:
> +       of_node_put(endpoint);
> +       return -ENODEV;
> +}
> +
> +static void ams495qa01_shutdown(struct spi_device *spi)
> +{
> +       struct ams495qa01 *db = spi_get_drvdata(spi);
> +       int ret;
> +
> +       ret = drm_panel_unprepare(&db->panel);
> +       if (ret < 0)
> +               dev_err(&spi->dev, "Failed to unprepare panel: %d\n", ret);
> +
> +       ret = drm_panel_disable(&db->panel);
> +       if (ret < 0)
> +               dev_err(&spi->dev, "Failed to disable panel: %d\n", ret);
> +}
> +
> +static void ams495qa01_remove(struct spi_device *spi)
> +{
> +       struct ams495qa01 *db = spi_get_drvdata(spi);
> +
> +       ams495qa01_shutdown(spi);
> +
> +       drm_panel_remove(&db->panel);
> +}
> +
> +static const struct of_device_id ams495qa01_match[] = {
> +       { .compatible = "samsung,ams495qa01", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, ams495qa01_match);
> +
> +static const struct spi_device_id ams495qa01_ids[] = {
> +       { "ams495qa01", 0 },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(spi, ams495qa01_ids);
> +
> +static struct spi_driver ams495qa01_driver = {
> +       .driver         = {
> +               .name   = "ams495qa01-panel",
> +               .of_match_table = ams495qa01_match,
> +       },
> +       .id_table       = ams495qa01_ids,
> +       .probe          = ams495qa01_probe,
> +       .remove         = ams495qa01_remove,
> +       .shutdown       = ams495qa01_shutdown,
> +};
> +module_spi_driver(ams495qa01_driver);
> +
> +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> +MODULE_DESCRIPTION("Samsung ams495qa01 panel driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>

Best Regards,
Maya Matuszczyk

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

* Re: [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
  2022-09-20 17:09   ` Chris Morgan
@ 2022-09-20 17:38     ` Maya Matuszczyk
  -1 siblings, 0 replies; 14+ messages in thread
From: Maya Matuszczyk @ 2022-09-20 17:38 UTC (permalink / raw)
  To: Chris Morgan
  Cc: dri-devel, devicetree, krzysztof.kozlowski+dt, airlied,
	Chris Morgan, robh+dt, thierry.reding, sam

Hello,
Thanks for the patch,
It's so nice seeing more pieces of RG503 being upstreamed.

wt., 20 wrz 2022 o 19:10 Chris Morgan <macroalpha82@gmail.com> napisał(a):
>
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add documentation for the Samsung AMS495QA01 panel.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../display/panel/samsung,ams495qa01.yaml     | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> new file mode 100644
> index 000000000000..08358cdad19c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung AMS495QA01 4.95in 960x544 DSI/SPI panel
> +
> +maintainers:
> +  - Chris Morgan <macromorgan@hotmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: samsung,ams495qa01
> +  reg: true
> +  reset-gpios: true
> +  elvdd-supply:
> +    description: regulator that supplies voltage to the panel display
Datasheet says about that pin:
"Power pin for module analog", maybe in description that could be
accounted for?

> +  enable-gpios: true
> +  port: true
> +  vdd-supply:
> +    description: regulator that supplies voltage to panel logic
Maybe change that to "panel's logic"?

> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        panel@0 {
> +            compatible = "samsung,ams495qa01";
> +            reg = <0>;
> +            vdd-supply = <&vcc3v3_lcd0_n>;
> +        };
Where's the node of DSI controller the panel's attached to?

> +    };
> +
> +...
> --
> 2.25.1
>

Best Regards,
Maya Matuszczyk

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

* Re: [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
@ 2022-09-20 17:38     ` Maya Matuszczyk
  0 siblings, 0 replies; 14+ messages in thread
From: Maya Matuszczyk @ 2022-09-20 17:38 UTC (permalink / raw)
  To: Chris Morgan
  Cc: devicetree, krzysztof.kozlowski+dt, airlied, dri-devel, robh+dt,
	thierry.reding, Chris Morgan, sam

Hello,
Thanks for the patch,
It's so nice seeing more pieces of RG503 being upstreamed.

wt., 20 wrz 2022 o 19:10 Chris Morgan <macroalpha82@gmail.com> napisał(a):
>
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add documentation for the Samsung AMS495QA01 panel.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../display/panel/samsung,ams495qa01.yaml     | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> new file mode 100644
> index 000000000000..08358cdad19c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung AMS495QA01 4.95in 960x544 DSI/SPI panel
> +
> +maintainers:
> +  - Chris Morgan <macromorgan@hotmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: samsung,ams495qa01
> +  reg: true
> +  reset-gpios: true
> +  elvdd-supply:
> +    description: regulator that supplies voltage to the panel display
Datasheet says about that pin:
"Power pin for module analog", maybe in description that could be
accounted for?

> +  enable-gpios: true
> +  port: true
> +  vdd-supply:
> +    description: regulator that supplies voltage to panel logic
Maybe change that to "panel's logic"?

> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        panel@0 {
> +            compatible = "samsung,ams495qa01";
> +            reg = <0>;
> +            vdd-supply = <&vcc3v3_lcd0_n>;
> +        };
Where's the node of DSI controller the panel's attached to?

> +    };
> +
> +...
> --
> 2.25.1
>

Best Regards,
Maya Matuszczyk

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

* Re: [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
  2022-09-20 17:33     ` Maya Matuszczyk
@ 2022-09-20 21:03       ` Chris Morgan
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 21:03 UTC (permalink / raw)
  To: Maya Matuszczyk
  Cc: Chris Morgan, dri-devel, devicetree, krzysztof.kozlowski+dt,
	airlied, robh+dt, thierry.reding, sam

On Tue, Sep 20, 2022 at 07:33:00PM +0200, Maya Matuszczyk wrote:
> Hi Chris,
> Thanks for this patch,
> 
> wt., 20 wrz 2022 o 19:10 Chris Morgan <macroalpha82@gmail.com> napisał(a):
> >
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Support Samsung AMS495QA01 panel as found on the Anbernic RG503. Note
> > This panel receives video signals via DSI, however it receives
> > commands via 3-wire SPI.
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  drivers/gpu/drm/panel/Kconfig                 |  10 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
> >  3 files changed, 516 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index a9043eacce97..954b66a2adee 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -444,6 +444,16 @@ config DRM_PANEL_RONBO_RB070D30
> >           Say Y here if you want to enable support for Ronbo Electronics
> >           RB070D30 1024x600 DSI panel.
> >
> > +config DRM_PANEL_SAMSUNG_AMS495QA01
> > +       tristate "Samsung AMS495QA01 DSI panel"
> > +       depends on OF && SPI
> > +       depends on DRM_MIPI_DSI
> > +       select DRM_MIPI_DBI
> > +       help
> > +         DRM panel driver for the Samsung AMS495QA01 panel. This panel
> > +         receives video data via DSI but commands via 3-Wire 9-bit
> > +         SPI.
> > +
> >  config DRM_PANEL_SAMSUNG_ATNA33XC20
> >         tristate "Samsung ATNA33XC20 eDP panel"
> >         depends on OF
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index 34e717382dbb..de0b57baf851 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen
> >  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
> >  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
> >  obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
> > +obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS495QA01) += panel-samsung-ams495qa01.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> > new file mode 100644
> > index 000000000000..d693ba5f20c9
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> > @@ -0,0 +1,505 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Samsung ams495qa01 MIPI-DSI panel driver
> > + * Copyright (C) 2022 Chris Morgan
> > + */
> > +
> > +#include <drm/drm_mipi_dbi.h>
> > +#include <drm/drm_mipi_dsi.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_panel.h>
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/media-bus-format.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <video/mipi_display.h>
> > +
> > +struct ams495qa01 {
> > +       /** @dev: the container device */
> > +       struct device *dev;
> > +       /** @dbi: the DBI bus abstraction handle */
> > +       struct mipi_dbi dbi;
> > +       /** @panel: the DRM panel instance for this device */
> > +       struct drm_panel panel;
> > +       /** @reset: reset GPIO line */
> > +       struct gpio_desc *reset;
> > +       /** @enable: enable GPIO line */
> > +       struct gpio_desc *enable;
> > +       /** @reg_vdd: VDD supply regulator for panel logic */
> > +       struct regulator *reg_vdd;
> > +       /** @reg_elvdd: ELVDD supply regulator for panel display */
> > +       struct regulator *reg_elvdd;
> > +       /** @dsi_dev: DSI child device (panel) */
> > +       struct mipi_dsi_device *dsi_dev;
> > +       /** @bl_dev: pseudo-backlight device for oled panel */
> > +       struct backlight_device *bl_dev;
> > +       /** @prepared: value tracking panel prepare status */
> > +       bool prepared;
> > +};
> > +
> > +static const struct drm_display_mode ams495qa01_mode = {
> > +       .clock = 33500,
> > +       .hdisplay = 960,
> > +       .hsync_start = 960 + 10,
> > +       .hsync_end = 960 + 10 + 2,
> > +       .htotal = 960 + 10 + 2 + 10,
> > +       .vdisplay = 544,
> > +       .vsync_start = 544 + 10,
> > +       .vsync_end = 544 + 10 + 2,
> > +       .vtotal = 544 + 10 + 2 + 10,
> > +       .width_mm = 117,
> > +       .height_mm = 74,
> > +       .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > +};
> > +
> > +#define NUM_GAMMA_LEVELS       16
> > +#define GAMMA_TABLE_COUNT      23
> > +#define MAX_BRIGHTNESS         (NUM_GAMMA_LEVELS - 1)
> > +
> > +/* Table of gamma values provided in datasheet */
> > +static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
> > +       {0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
> > +        0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
> > +        0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
> > +        0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
> > +        0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
> > +        0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
> > +        0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
> > +        0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
> > +        0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
> > +        0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
> > +        0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
> > +        0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
> > +        0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
> > +        0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
> > +        0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
> > +        0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
> > +        0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> There might be some use in adding unofficial gamma values,
> for higher and lower brightness values than officially supported,
> For example, vitabright repository of devnoname120 contains
> gamma lookup tables that should be compatible with this
> display.
> Perhaps it's worth checking out, for low-light and outdoors
> use case, as the display is meant to be used in handheld
> gaming devices:
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdevnoname120%2Fvitabright&amp;data=05%7C01%7C%7Cb6531a6abc1940709ccd08da9b2e3fbc%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637992920188081853%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=KColbBBN2hg6c3L%2F7XuXkIBWQxoxoaHE9WxcEXP%2BoKw%3D&amp;reserved=0

Personally I'm against adding additional gamma values, since the values
listed in here come directly from the panel's datasheet. I'll let the
drm folks chime in though. Maybe worst case we could have a module
parameter for additional gamma values.

> 
> > +};
> > +
> > +/*
> > + * Table of elvss values provided in datasheet and corresponds to
> > + * gamma values.
> > + */
> > +static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
> > +       0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
> > +       0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
> > +};
> > +
> > +static inline struct ams495qa01 *to_ams495qa01(struct drm_panel *panel)
> > +{
> > +       return container_of(panel, struct ams495qa01, panel);
> > +}
> > +
> > +static int ams495qa01_update_gamma(struct mipi_dbi *dbi, u32 brightness)
> > +{
> > +       u32 tmp = brightness;
> > +
> > +       if (brightness > MAX_BRIGHTNESS)
> > +               tmp = MAX_BRIGHTNESS;
> Shouldn't we return -EINVAL in this case?
> 

I looked at a few example drivers that had built-in backlights
(this being an oled panel it has a "backlight") but actually
didn't find any examples on how it handled values larger than
it can support. I'm open to either dropping it to the max
or returning -EINVAL, I guess it just depends on what DRM
maintainers prefer.

> > +
> > +       /* Set gamma values */
> > +       mipi_dbi_command_buf(dbi, 0xf9, ams495qa01_gamma[tmp],
> > +                            ARRAY_SIZE(ams495qa01_gamma[tmp]));
> > +       /* Set gamma change */
> > +       mipi_dbi_command(dbi, 0xf9, 0x00);
> > +       /* Undocumented command */
> > +       mipi_dbi_command(dbi, 0x26, 0x00);
> > +       /* Set ELVSS value */
> > +       mipi_dbi_command(dbi, 0xb2, ams495qa01_elvss[tmp]);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ams495qa01_prepare(struct drm_panel *panel)
> > +{
> > +       struct ams495qa01 *db = to_ams495qa01(panel);
> > +       struct mipi_dbi *dbi = &db->dbi;
> > +       int ret;
> > +
> > +       if (db->prepared)
> > +               return 0;
> > +
> > +       /* Power up */
> > +       ret = regulator_enable(db->reg_vdd);
> > +       if (ret) {
> > +               dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
> > +               return ret;
> > +       }
> > +       if (db->reg_elvdd) {
> > +               ret = regulator_enable(db->reg_elvdd);
> > +               if (ret) {
> > +                       dev_err(db->dev,
> > +                               "failed to enable elvdd regulator: %d\n", ret);
> > +                       regulator_disable(db->reg_vdd);
> > +                       return ret;
> > +               }
> > +       }
> Maybe ret could be initialized with 0 value, and couple gotos like
> with some other drivers?
> 

Since this is the only point where we return after one or more
regulators are enabled a goto isn't necessary here... yet. If
more error checking is required than one might be necessary.

> > +
> > +       /* Enable */
> > +       if (db->enable)
> > +               gpiod_set_value_cansleep(db->enable, 1);
> > +
> > +       msleep(50);
> > +
> > +       /* Reset */
> > +       gpiod_set_value_cansleep(db->reset, 1);
> > +       usleep_range(1000, 5000);
> > +       gpiod_set_value_cansleep(db->reset, 0);
> > +       msleep(20);
> > +
> > +       /* Panel Init Sequence */
> > +
> > +       /* Password to start command sequence */
> > +       mipi_dbi_command(dbi, 0xf0, 0x5a, 0x5a);
> > +       mipi_dbi_command(dbi, 0xf1, 0x5a, 0x5a);
> > +
> > +       /* Undocumented commands */
> > +       mipi_dbi_command(dbi, 0xb0, 0x02);
> > +       mipi_dbi_command(dbi, 0xf3, 0x3b);
> > +
> > +       /* Analog Power condition set */
> > +       mipi_dbi_command(dbi, 0xf4, 0x33, 0x42, 0x00, 0x08);
> > +       mipi_dbi_command(dbi, 0xf5, 0x00, 0x06, 0x26, 0x35, 0x03);
> > +
> > +       /* Undocumented commands */
> > +       mipi_dbi_command(dbi, 0xf6, 0x02);
> > +       mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
> > +                        0x00, 0x00, 0x00, 0x00);
> > +
> > +       /* GTCON set */
> > +       mipi_dbi_command(dbi, 0xf7, 0x20);
> > +
> > +       /* TEMP_SWIRE set */
> > +       mipi_dbi_command(dbi, 0xb2, 0x06, 0x06, 0x06, 0x06);
> > +
> > +       /* ELVSS_CON set */
> > +       mipi_dbi_command(dbi, 0xb1, 0x07, 0x00, 0x10);
> > +
> > +       /* Gateless signal set */
> > +       mipi_dbi_command(dbi, 0xf8, 0x7f, 0x7a, 0x89, 0x67, 0x26, 0x38,
> > +                        0x00, 0x00, 0x09, 0x67, 0x70, 0x88, 0x7a,
> > +                        0x76, 0x05, 0x09, 0x23, 0x23, 0x23);
> > +
> > +       /* Undocumented commands */
> > +       mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
> > +                        0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
> > +                        0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
> > +                        0xff);
> > +       mipi_dbi_command(dbi, 0xb4, 0x15);
> > +       mipi_dbi_command(dbi, 0xb3, 0x00);
> > +
> > +       ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
> > +
> > +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> > +       msleep(200);
> > +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
> > +       usleep_range(10000, 15000);
> > +
> > +       db->prepared = true;
> > +
> > +       return 0;
> > +}
> > +
> > +static int ams495qa01_unprepare(struct drm_panel *panel)
> > +{
> > +       struct ams495qa01 *db = to_ams495qa01(panel);
> > +       struct mipi_dbi *dbi = &db->dbi;
> > +
> > +       if (!db->prepared)
> > +               return 0;
> > +
> > +       /* Panel Exit Sequence */
> > +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> > +       msleep(20);
> > +       mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
> > +       usleep_range(10000, 15000);
> > +
> > +       if (db->enable)
> > +               gpiod_set_value_cansleep(db->enable, 0);
> > +       if (db->reg_elvdd)
> > +               regulator_disable(db->reg_elvdd);
> > +       regulator_disable(db->reg_vdd);
> > +       msleep(20);
> > +
> > +       db->prepared = false;
> > +
> > +       return 0;
> > +}
> > +
> > +static int ams495qa01_get_modes(struct drm_panel *panel,
> > +                               struct drm_connector *connector)
> > +{
> > +       struct ams495qa01 *db = to_ams495qa01(panel);
> > +       struct drm_display_mode *mode;
> > +       static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > +
> > +       mode = drm_mode_duplicate(connector->dev, &ams495qa01_mode);
> > +       if (!mode) {
> > +               dev_err(db->dev, "failed to add mode\n");
> > +               return -ENOMEM;
> > +       }
> > +
> > +       connector->display_info.bpc = 8;
> > +       connector->display_info.width_mm = mode->width_mm;
> > +       connector->display_info.height_mm = mode->height_mm;
> > +       connector->display_info.bus_flags =
> > +               DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
> > +               DRM_BUS_FLAG_DE_LOW;
> > +       drm_display_info_set_bus_formats(&connector->display_info,
> > +                                        &bus_format, 1);
> > +
> > +       drm_mode_set_name(mode);
> > +       mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> > +
> > +       drm_mode_probed_add(connector, mode);
> > +
> > +       return 1;
> > +}
> > +
> > +static const struct drm_panel_funcs ams495qa01_drm_funcs = {
> > +       .unprepare = ams495qa01_unprepare,
> > +       .prepare = ams495qa01_prepare,
> > +       .get_modes = ams495qa01_get_modes,
> > +};
> > +
> > +static int ams495qa01_set_brightness(struct backlight_device *bd)
> > +{
> > +       struct ams495qa01 *db = bl_get_data(bd);
> > +       struct mipi_dbi *dbi = &db->dbi;
> > +       int brightness = bd->props.brightness;
> > +
> > +       ams495qa01_update_gamma(dbi, brightness);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct backlight_ops ams495qa01_backlight_ops = {
> > +       .update_status  = ams495qa01_set_brightness,
> > +};
> > +
> > +static int ams495qa01_backlight_register(struct ams495qa01 *db)
> > +{
> > +       struct backlight_properties props = {
> > +               .type           = BACKLIGHT_RAW,
> > +               .brightness     = MAX_BRIGHTNESS,
> > +               .max_brightness = MAX_BRIGHTNESS,
> > +       };
> > +       struct device *dev = db->dev;
> > +       int ret = 0;
> > +
> > +       db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
> > +                                                    &ams495qa01_backlight_ops,
> > +                                                    &props);
> > +       if (IS_ERR(db->bl_dev)) {
> > +               ret = PTR_ERR(db->bl_dev);
> > +               dev_err(dev, "error registering backlight device (%d)\n", ret);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int ams495qa01_probe(struct spi_device *spi)
> > +{
> > +       struct device *dev = &spi->dev;
> > +       struct device_node *endpoint, *dsi_host_node;
> > +       struct mipi_dsi_host *dsi_host;
> > +       struct ams495qa01 *db;
> > +       int ret;
> > +       struct mipi_dsi_device_info info = {
> > +               .type = "dupa",
> You might want to change this to something more appropiate for Linux.
> 
> I see this was left from a prototype driver that I wrote, which you might
> have based this one on.

Yep, guilty as charged. As said on Discord this driver is about 20%
you, 25% me, and 55% copied and pasted from other drivers. I'll change
this so it's not a swear word in Polish. My swearing ability is
limited to English and Spanish which is why I didn't catch it. :-p

I'll make sure to include you on the signed-off for v3 since this is
somewhat of a joint project.

> 
> > +               .channel = 0,
> > +               .node = NULL,
> > +       };
> > +
> > +       db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
> > +       if (!db)
> > +               return -ENOMEM;
> > +
> > +       spi_set_drvdata(spi, db);
> > +
> > +       db->dev = dev;
> > +
> > +       db->reg_vdd = devm_regulator_get(dev, "vdd");
> > +       if (IS_ERR(db->reg_vdd))
> > +               return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
> > +                                    "Failed to get vdd supply\n");
> > +
> > +       db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
> > +       if (IS_ERR(db->reg_elvdd))
> > +               db->reg_elvdd = NULL;
> > +
> > +       db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > +       if (IS_ERR(db->reset)) {
> > +               ret = PTR_ERR(db->reset);
> > +               return dev_err_probe(dev, ret, "no RESET GPIO\n");
> I think this can be shortened into:
> return dev_err_probe(dev, PTR_ERR(db->reset), "cannot get reset gpio\n");
> 

It can, but I think it boils down to preference. To me it's easier to
read as it's written, but I'll let the subsystem maintainers chime in.

> > +       }
> > +
> > +       db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> > +       if (IS_ERR(db->enable)) {
> > +               ret = PTR_ERR(db->enable);
> > +               return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
> > +       }
> > +
> > +       ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
> It's not always controlled via SPI, sometimes(based on a strap pin)
> it's controlled via MIPI-DSI commands.
> 
> Maybe let's either have another DT compatible for that?
> I'm really not sure

The panel-samsung-s6e63m0 driver is probably a good example here on how
to handle an SPI/DSI display maybe? I'm not aware of any other devices
with this panel used in pure DSI mode though and have no such device on
which to test. For my device the strap is hard-wired to force SPI only
command mode. What differs here is that the s6e63m0 driver is either
SPI 3-Wire + DPI or DSI, this one is either SPI 3-Wire + DSI or DSI.
I don't know if that means we can simplify into a single driver or have
to split it like the s6e63m0. Either way, I'm not comfortable commiting
code I have no way to test.

> 
> > +       if (ret)
> > +               return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> > +
> > +       /*
> > +        * Get the DSI controller that is supplying data for this display
> > +        * which is controlled via SPI 3-wire.
> Same as above
> 
> > +        */
> > +       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > +       if (!endpoint) {
> > +               dev_err(dev, "failed to get endpoint\n");
> > +               return -ENODEV;
> > +       }
> > +       dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > +       if (!dsi_host_node) {
> > +               dev_err(dev, "failed to get remote port parent\n");
> > +               goto put_endpoint;
> > +       }
> > +       dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > +       if (!dsi_host) {
> > +               dev_err(dev, "failed to find dsi host\n");
> > +               goto put_host;
> > +       }
> > +       info.node = of_graph_get_remote_port(endpoint);
> > +       if (!info.node) {
> > +               dev_err(dev, "failed to get remote port node\n");
> > +               ret = -ENODEV;
> > +               goto put_host;
> > +       }
> > +
> > +       db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
> > +       if (IS_ERR(db->dsi_dev)) {
> > +               dev_err(dev, "failed to register dsi device: %ld\n",
> > +                       PTR_ERR(db->dsi_dev));
> > +               ret = PTR_ERR(db->dsi_dev);
> > +               goto put_host;
> > +       }
> > +
> > +       db->dsi_dev->lanes = 2;
> > +       db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
> > +       db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > +                         MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> > +
> > +       drm_panel_init(&db->panel, dev, &ams495qa01_drm_funcs,
> > +                      DRM_MODE_CONNECTOR_DSI);
> > +
> > +       ret = ams495qa01_backlight_register(db);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       drm_panel_add(&db->panel);
> > +
> > +       ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
> > +       if (ret < 0) {
> > +               dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
> > +               drm_panel_remove(&db->panel);
> > +               return ret;
> > +       }
> > +
> > +       of_node_put(dsi_host_node);
> > +       of_node_put(endpoint);
> > +
> > +       return 0;
> > +
> > +put_host:
> > +       of_node_put(dsi_host_node);
> > +
> > +put_endpoint:
> > +       of_node_put(endpoint);
> > +       return -ENODEV;
> > +}
> > +
> > +static void ams495qa01_shutdown(struct spi_device *spi)
> > +{
> > +       struct ams495qa01 *db = spi_get_drvdata(spi);
> > +       int ret;
> > +
> > +       ret = drm_panel_unprepare(&db->panel);
> > +       if (ret < 0)
> > +               dev_err(&spi->dev, "Failed to unprepare panel: %d\n", ret);
> > +
> > +       ret = drm_panel_disable(&db->panel);
> > +       if (ret < 0)
> > +               dev_err(&spi->dev, "Failed to disable panel: %d\n", ret);
> > +}
> > +
> > +static void ams495qa01_remove(struct spi_device *spi)
> > +{
> > +       struct ams495qa01 *db = spi_get_drvdata(spi);
> > +
> > +       ams495qa01_shutdown(spi);
> > +
> > +       drm_panel_remove(&db->panel);
> > +}
> > +
> > +static const struct of_device_id ams495qa01_match[] = {
> > +       { .compatible = "samsung,ams495qa01", },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ams495qa01_match);
> > +
> > +static const struct spi_device_id ams495qa01_ids[] = {
> > +       { "ams495qa01", 0 },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(spi, ams495qa01_ids);
> > +
> > +static struct spi_driver ams495qa01_driver = {
> > +       .driver         = {
> > +               .name   = "ams495qa01-panel",
> > +               .of_match_table = ams495qa01_match,
> > +       },
> > +       .id_table       = ams495qa01_ids,
> > +       .probe          = ams495qa01_probe,
> > +       .remove         = ams495qa01_remove,
> > +       .shutdown       = ams495qa01_shutdown,
> > +};
> > +module_spi_driver(ams495qa01_driver);
> > +
> > +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> > +MODULE_DESCRIPTION("Samsung ams495qa01 panel driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
> 
> Best Regards,
> Maya Matuszczyk

As always appreciate the feedback and enjoy working with you.

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

* Re: [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
@ 2022-09-20 21:03       ` Chris Morgan
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Morgan @ 2022-09-20 21:03 UTC (permalink / raw)
  To: Maya Matuszczyk
  Cc: devicetree, airlied, dri-devel, Chris Morgan, robh+dt,
	thierry.reding, krzysztof.kozlowski+dt, sam

On Tue, Sep 20, 2022 at 07:33:00PM +0200, Maya Matuszczyk wrote:
> Hi Chris,
> Thanks for this patch,
> 
> wt., 20 wrz 2022 o 19:10 Chris Morgan <macroalpha82@gmail.com> napisał(a):
> >
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Support Samsung AMS495QA01 panel as found on the Anbernic RG503. Note
> > This panel receives video signals via DSI, however it receives
> > commands via 3-wire SPI.
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  drivers/gpu/drm/panel/Kconfig                 |  10 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../gpu/drm/panel/panel-samsung-ams495qa01.c  | 505 ++++++++++++++++++
> >  3 files changed, 516 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index a9043eacce97..954b66a2adee 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -444,6 +444,16 @@ config DRM_PANEL_RONBO_RB070D30
> >           Say Y here if you want to enable support for Ronbo Electronics
> >           RB070D30 1024x600 DSI panel.
> >
> > +config DRM_PANEL_SAMSUNG_AMS495QA01
> > +       tristate "Samsung AMS495QA01 DSI panel"
> > +       depends on OF && SPI
> > +       depends on DRM_MIPI_DSI
> > +       select DRM_MIPI_DBI
> > +       help
> > +         DRM panel driver for the Samsung AMS495QA01 panel. This panel
> > +         receives video data via DSI but commands via 3-Wire 9-bit
> > +         SPI.
> > +
> >  config DRM_PANEL_SAMSUNG_ATNA33XC20
> >         tristate "Samsung ATNA33XC20 eDP panel"
> >         depends on OF
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index 34e717382dbb..de0b57baf851 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen
> >  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
> >  obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
> >  obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
> > +obj-$(CONFIG_DRM_PANEL_SAMSUNG_AMS495QA01) += panel-samsung-ams495qa01.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> > new file mode 100644
> > index 000000000000..d693ba5f20c9
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ams495qa01.c
> > @@ -0,0 +1,505 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Samsung ams495qa01 MIPI-DSI panel driver
> > + * Copyright (C) 2022 Chris Morgan
> > + */
> > +
> > +#include <drm/drm_mipi_dbi.h>
> > +#include <drm/drm_mipi_dsi.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_panel.h>
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/media-bus-format.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <video/mipi_display.h>
> > +
> > +struct ams495qa01 {
> > +       /** @dev: the container device */
> > +       struct device *dev;
> > +       /** @dbi: the DBI bus abstraction handle */
> > +       struct mipi_dbi dbi;
> > +       /** @panel: the DRM panel instance for this device */
> > +       struct drm_panel panel;
> > +       /** @reset: reset GPIO line */
> > +       struct gpio_desc *reset;
> > +       /** @enable: enable GPIO line */
> > +       struct gpio_desc *enable;
> > +       /** @reg_vdd: VDD supply regulator for panel logic */
> > +       struct regulator *reg_vdd;
> > +       /** @reg_elvdd: ELVDD supply regulator for panel display */
> > +       struct regulator *reg_elvdd;
> > +       /** @dsi_dev: DSI child device (panel) */
> > +       struct mipi_dsi_device *dsi_dev;
> > +       /** @bl_dev: pseudo-backlight device for oled panel */
> > +       struct backlight_device *bl_dev;
> > +       /** @prepared: value tracking panel prepare status */
> > +       bool prepared;
> > +};
> > +
> > +static const struct drm_display_mode ams495qa01_mode = {
> > +       .clock = 33500,
> > +       .hdisplay = 960,
> > +       .hsync_start = 960 + 10,
> > +       .hsync_end = 960 + 10 + 2,
> > +       .htotal = 960 + 10 + 2 + 10,
> > +       .vdisplay = 544,
> > +       .vsync_start = 544 + 10,
> > +       .vsync_end = 544 + 10 + 2,
> > +       .vtotal = 544 + 10 + 2 + 10,
> > +       .width_mm = 117,
> > +       .height_mm = 74,
> > +       .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > +};
> > +
> > +#define NUM_GAMMA_LEVELS       16
> > +#define GAMMA_TABLE_COUNT      23
> > +#define MAX_BRIGHTNESS         (NUM_GAMMA_LEVELS - 1)
> > +
> > +/* Table of gamma values provided in datasheet */
> > +static u8 ams495qa01_gamma[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
> > +       {0x01, 0x79, 0x78, 0x8d, 0xd9, 0xdf, 0xd5, 0xcb, 0xcf, 0xc5,
> > +        0xe5, 0xe0, 0xe4, 0xdc, 0xb8, 0xd4, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x7d, 0x7c, 0x92, 0xd7, 0xdd, 0xd2, 0xcb, 0xd0, 0xc6,
> > +        0xe5, 0xe1, 0xe3, 0xda, 0xbd, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x7f, 0x7e, 0x95, 0xd7, 0xde, 0xd2, 0xcb, 0xcf, 0xc5,
> > +        0xe5, 0xe3, 0xe3, 0xda, 0xbf, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x82, 0x81, 0x99, 0xd6, 0xdd, 0xd1, 0xca, 0xcf, 0xc3,
> > +        0xe4, 0xe3, 0xe3, 0xda, 0xc2, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x84, 0x83, 0x9b, 0xd7, 0xde, 0xd2, 0xc8, 0xce, 0xc2,
> > +        0xe4, 0xe3, 0xe2, 0xd9, 0xc3, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x87, 0x86, 0x9f, 0xd6, 0xdd, 0xd1, 0xc7, 0xce, 0xc1,
> > +        0xe4, 0xe3, 0xe2, 0xd9, 0xc6, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x89, 0x89, 0xa2, 0xd5, 0xdb, 0xcf, 0xc8, 0xcf, 0xc2,
> > +        0xe3, 0xe3, 0xe1, 0xd9, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x8b, 0x8b, 0xa5, 0xd5, 0xdb, 0xcf, 0xc7, 0xce, 0xc0,
> > +        0xe3, 0xe3, 0xe1, 0xd8, 0xc7, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x8d, 0x8d, 0xa7, 0xd5, 0xdb, 0xcf, 0xc6, 0xce, 0xc0,
> > +        0xe4, 0xe4, 0xe1, 0xd7, 0xc8, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x8f, 0x8f, 0xaa, 0xd4, 0xdb, 0xce, 0xc6, 0xcd, 0xbf,
> > +        0xe3, 0xe3, 0xe1, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x91, 0x91, 0xac, 0xd3, 0xda, 0xce, 0xc5, 0xcd, 0xbe,
> > +        0xe3, 0xe3, 0xe0, 0xd7, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x93, 0x93, 0xaf, 0xd3, 0xda, 0xcd, 0xc5, 0xcd, 0xbe,
> > +        0xe2, 0xe3, 0xdf, 0xd6, 0xca, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x95, 0x95, 0xb1, 0xd2, 0xd9, 0xcc, 0xc4, 0xcd, 0xbe,
> > +        0xe2, 0xe3, 0xdf, 0xd7, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x99, 0x99, 0xb6, 0xd1, 0xd9, 0xcc, 0xc3, 0xcb, 0xbc,
> > +        0xe2, 0xe4, 0xdf, 0xd6, 0xcc, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x9c, 0x9c, 0xba, 0xd0, 0xd8, 0xcb, 0xc3, 0xcb, 0xbb,
> > +        0xe2, 0xe4, 0xdf, 0xd6, 0xce, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> > +       {0x01, 0x9f, 0x9f, 0xbe, 0xcf, 0xd7, 0xc9, 0xc2, 0xcb, 0xbb,
> > +        0xe1, 0xe3, 0xde, 0xd6, 0xd0, 0xd3, 0xfa, 0xed, 0xe6, 0x2f,
> > +        0x00, 0x2f},
> There might be some use in adding unofficial gamma values,
> for higher and lower brightness values than officially supported,
> For example, vitabright repository of devnoname120 contains
> gamma lookup tables that should be compatible with this
> display.
> Perhaps it's worth checking out, for low-light and outdoors
> use case, as the display is meant to be used in handheld
> gaming devices:
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdevnoname120%2Fvitabright&amp;data=05%7C01%7C%7Cb6531a6abc1940709ccd08da9b2e3fbc%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637992920188081853%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=KColbBBN2hg6c3L%2F7XuXkIBWQxoxoaHE9WxcEXP%2BoKw%3D&amp;reserved=0

Personally I'm against adding additional gamma values, since the values
listed in here come directly from the panel's datasheet. I'll let the
drm folks chime in though. Maybe worst case we could have a module
parameter for additional gamma values.

> 
> > +};
> > +
> > +/*
> > + * Table of elvss values provided in datasheet and corresponds to
> > + * gamma values.
> > + */
> > +static u8 ams495qa01_elvss[NUM_GAMMA_LEVELS] = {
> > +       0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15, 0x15,
> > +       0x15, 0x15, 0x14, 0x14, 0x13, 0x12,
> > +};
> > +
> > +static inline struct ams495qa01 *to_ams495qa01(struct drm_panel *panel)
> > +{
> > +       return container_of(panel, struct ams495qa01, panel);
> > +}
> > +
> > +static int ams495qa01_update_gamma(struct mipi_dbi *dbi, u32 brightness)
> > +{
> > +       u32 tmp = brightness;
> > +
> > +       if (brightness > MAX_BRIGHTNESS)
> > +               tmp = MAX_BRIGHTNESS;
> Shouldn't we return -EINVAL in this case?
> 

I looked at a few example drivers that had built-in backlights
(this being an oled panel it has a "backlight") but actually
didn't find any examples on how it handled values larger than
it can support. I'm open to either dropping it to the max
or returning -EINVAL, I guess it just depends on what DRM
maintainers prefer.

> > +
> > +       /* Set gamma values */
> > +       mipi_dbi_command_buf(dbi, 0xf9, ams495qa01_gamma[tmp],
> > +                            ARRAY_SIZE(ams495qa01_gamma[tmp]));
> > +       /* Set gamma change */
> > +       mipi_dbi_command(dbi, 0xf9, 0x00);
> > +       /* Undocumented command */
> > +       mipi_dbi_command(dbi, 0x26, 0x00);
> > +       /* Set ELVSS value */
> > +       mipi_dbi_command(dbi, 0xb2, ams495qa01_elvss[tmp]);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ams495qa01_prepare(struct drm_panel *panel)
> > +{
> > +       struct ams495qa01 *db = to_ams495qa01(panel);
> > +       struct mipi_dbi *dbi = &db->dbi;
> > +       int ret;
> > +
> > +       if (db->prepared)
> > +               return 0;
> > +
> > +       /* Power up */
> > +       ret = regulator_enable(db->reg_vdd);
> > +       if (ret) {
> > +               dev_err(db->dev, "failed to enable vdd regulator: %d\n", ret);
> > +               return ret;
> > +       }
> > +       if (db->reg_elvdd) {
> > +               ret = regulator_enable(db->reg_elvdd);
> > +               if (ret) {
> > +                       dev_err(db->dev,
> > +                               "failed to enable elvdd regulator: %d\n", ret);
> > +                       regulator_disable(db->reg_vdd);
> > +                       return ret;
> > +               }
> > +       }
> Maybe ret could be initialized with 0 value, and couple gotos like
> with some other drivers?
> 

Since this is the only point where we return after one or more
regulators are enabled a goto isn't necessary here... yet. If
more error checking is required than one might be necessary.

> > +
> > +       /* Enable */
> > +       if (db->enable)
> > +               gpiod_set_value_cansleep(db->enable, 1);
> > +
> > +       msleep(50);
> > +
> > +       /* Reset */
> > +       gpiod_set_value_cansleep(db->reset, 1);
> > +       usleep_range(1000, 5000);
> > +       gpiod_set_value_cansleep(db->reset, 0);
> > +       msleep(20);
> > +
> > +       /* Panel Init Sequence */
> > +
> > +       /* Password to start command sequence */
> > +       mipi_dbi_command(dbi, 0xf0, 0x5a, 0x5a);
> > +       mipi_dbi_command(dbi, 0xf1, 0x5a, 0x5a);
> > +
> > +       /* Undocumented commands */
> > +       mipi_dbi_command(dbi, 0xb0, 0x02);
> > +       mipi_dbi_command(dbi, 0xf3, 0x3b);
> > +
> > +       /* Analog Power condition set */
> > +       mipi_dbi_command(dbi, 0xf4, 0x33, 0x42, 0x00, 0x08);
> > +       mipi_dbi_command(dbi, 0xf5, 0x00, 0x06, 0x26, 0x35, 0x03);
> > +
> > +       /* Undocumented commands */
> > +       mipi_dbi_command(dbi, 0xf6, 0x02);
> > +       mipi_dbi_command(dbi, 0xc6, 0x0b, 0x00, 0x00, 0x3c, 0x00, 0x22,
> > +                        0x00, 0x00, 0x00, 0x00);
> > +
> > +       /* GTCON set */
> > +       mipi_dbi_command(dbi, 0xf7, 0x20);
> > +
> > +       /* TEMP_SWIRE set */
> > +       mipi_dbi_command(dbi, 0xb2, 0x06, 0x06, 0x06, 0x06);
> > +
> > +       /* ELVSS_CON set */
> > +       mipi_dbi_command(dbi, 0xb1, 0x07, 0x00, 0x10);
> > +
> > +       /* Gateless signal set */
> > +       mipi_dbi_command(dbi, 0xf8, 0x7f, 0x7a, 0x89, 0x67, 0x26, 0x38,
> > +                        0x00, 0x00, 0x09, 0x67, 0x70, 0x88, 0x7a,
> > +                        0x76, 0x05, 0x09, 0x23, 0x23, 0x23);
> > +
> > +       /* Undocumented commands */
> > +       mipi_dbi_command(dbi, 0xb5, 0xff, 0xef, 0x35, 0x42, 0x0d, 0xd7,
> > +                        0xff, 0x07, 0xff, 0xff, 0xfd, 0x00, 0x01,
> > +                        0xff, 0x05, 0x12, 0x0f, 0xff, 0xff, 0xff,
> > +                        0xff);
> > +       mipi_dbi_command(dbi, 0xb4, 0x15);
> > +       mipi_dbi_command(dbi, 0xb3, 0x00);
> > +
> > +       ams495qa01_update_gamma(dbi, MAX_BRIGHTNESS);
> > +
> > +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> > +       msleep(200);
> > +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
> > +       usleep_range(10000, 15000);
> > +
> > +       db->prepared = true;
> > +
> > +       return 0;
> > +}
> > +
> > +static int ams495qa01_unprepare(struct drm_panel *panel)
> > +{
> > +       struct ams495qa01 *db = to_ams495qa01(panel);
> > +       struct mipi_dbi *dbi = &db->dbi;
> > +
> > +       if (!db->prepared)
> > +               return 0;
> > +
> > +       /* Panel Exit Sequence */
> > +       mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> > +       msleep(20);
> > +       mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
> > +       usleep_range(10000, 15000);
> > +
> > +       if (db->enable)
> > +               gpiod_set_value_cansleep(db->enable, 0);
> > +       if (db->reg_elvdd)
> > +               regulator_disable(db->reg_elvdd);
> > +       regulator_disable(db->reg_vdd);
> > +       msleep(20);
> > +
> > +       db->prepared = false;
> > +
> > +       return 0;
> > +}
> > +
> > +static int ams495qa01_get_modes(struct drm_panel *panel,
> > +                               struct drm_connector *connector)
> > +{
> > +       struct ams495qa01 *db = to_ams495qa01(panel);
> > +       struct drm_display_mode *mode;
> > +       static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > +
> > +       mode = drm_mode_duplicate(connector->dev, &ams495qa01_mode);
> > +       if (!mode) {
> > +               dev_err(db->dev, "failed to add mode\n");
> > +               return -ENOMEM;
> > +       }
> > +
> > +       connector->display_info.bpc = 8;
> > +       connector->display_info.width_mm = mode->width_mm;
> > +       connector->display_info.height_mm = mode->height_mm;
> > +       connector->display_info.bus_flags =
> > +               DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
> > +               DRM_BUS_FLAG_DE_LOW;
> > +       drm_display_info_set_bus_formats(&connector->display_info,
> > +                                        &bus_format, 1);
> > +
> > +       drm_mode_set_name(mode);
> > +       mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> > +
> > +       drm_mode_probed_add(connector, mode);
> > +
> > +       return 1;
> > +}
> > +
> > +static const struct drm_panel_funcs ams495qa01_drm_funcs = {
> > +       .unprepare = ams495qa01_unprepare,
> > +       .prepare = ams495qa01_prepare,
> > +       .get_modes = ams495qa01_get_modes,
> > +};
> > +
> > +static int ams495qa01_set_brightness(struct backlight_device *bd)
> > +{
> > +       struct ams495qa01 *db = bl_get_data(bd);
> > +       struct mipi_dbi *dbi = &db->dbi;
> > +       int brightness = bd->props.brightness;
> > +
> > +       ams495qa01_update_gamma(dbi, brightness);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct backlight_ops ams495qa01_backlight_ops = {
> > +       .update_status  = ams495qa01_set_brightness,
> > +};
> > +
> > +static int ams495qa01_backlight_register(struct ams495qa01 *db)
> > +{
> > +       struct backlight_properties props = {
> > +               .type           = BACKLIGHT_RAW,
> > +               .brightness     = MAX_BRIGHTNESS,
> > +               .max_brightness = MAX_BRIGHTNESS,
> > +       };
> > +       struct device *dev = db->dev;
> > +       int ret = 0;
> > +
> > +       db->bl_dev = devm_backlight_device_register(dev, "panel", dev, db,
> > +                                                    &ams495qa01_backlight_ops,
> > +                                                    &props);
> > +       if (IS_ERR(db->bl_dev)) {
> > +               ret = PTR_ERR(db->bl_dev);
> > +               dev_err(dev, "error registering backlight device (%d)\n", ret);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int ams495qa01_probe(struct spi_device *spi)
> > +{
> > +       struct device *dev = &spi->dev;
> > +       struct device_node *endpoint, *dsi_host_node;
> > +       struct mipi_dsi_host *dsi_host;
> > +       struct ams495qa01 *db;
> > +       int ret;
> > +       struct mipi_dsi_device_info info = {
> > +               .type = "dupa",
> You might want to change this to something more appropiate for Linux.
> 
> I see this was left from a prototype driver that I wrote, which you might
> have based this one on.

Yep, guilty as charged. As said on Discord this driver is about 20%
you, 25% me, and 55% copied and pasted from other drivers. I'll change
this so it's not a swear word in Polish. My swearing ability is
limited to English and Spanish which is why I didn't catch it. :-p

I'll make sure to include you on the signed-off for v3 since this is
somewhat of a joint project.

> 
> > +               .channel = 0,
> > +               .node = NULL,
> > +       };
> > +
> > +       db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
> > +       if (!db)
> > +               return -ENOMEM;
> > +
> > +       spi_set_drvdata(spi, db);
> > +
> > +       db->dev = dev;
> > +
> > +       db->reg_vdd = devm_regulator_get(dev, "vdd");
> > +       if (IS_ERR(db->reg_vdd))
> > +               return dev_err_probe(dev, PTR_ERR(db->reg_vdd),
> > +                                    "Failed to get vdd supply\n");
> > +
> > +       db->reg_elvdd = devm_regulator_get_optional(dev, "elvdd");
> > +       if (IS_ERR(db->reg_elvdd))
> > +               db->reg_elvdd = NULL;
> > +
> > +       db->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > +       if (IS_ERR(db->reset)) {
> > +               ret = PTR_ERR(db->reset);
> > +               return dev_err_probe(dev, ret, "no RESET GPIO\n");
> I think this can be shortened into:
> return dev_err_probe(dev, PTR_ERR(db->reset), "cannot get reset gpio\n");
> 

It can, but I think it boils down to preference. To me it's easier to
read as it's written, but I'll let the subsystem maintainers chime in.

> > +       }
> > +
> > +       db->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> > +       if (IS_ERR(db->enable)) {
> > +               ret = PTR_ERR(db->enable);
> > +               return dev_err_probe(dev, ret, "cannot get ENABLE GPIO\n");
> > +       }
> > +
> > +       ret = mipi_dbi_spi_init(spi, &db->dbi, NULL);
> It's not always controlled via SPI, sometimes(based on a strap pin)
> it's controlled via MIPI-DSI commands.
> 
> Maybe let's either have another DT compatible for that?
> I'm really not sure

The panel-samsung-s6e63m0 driver is probably a good example here on how
to handle an SPI/DSI display maybe? I'm not aware of any other devices
with this panel used in pure DSI mode though and have no such device on
which to test. For my device the strap is hard-wired to force SPI only
command mode. What differs here is that the s6e63m0 driver is either
SPI 3-Wire + DPI or DSI, this one is either SPI 3-Wire + DSI or DSI.
I don't know if that means we can simplify into a single driver or have
to split it like the s6e63m0. Either way, I'm not comfortable commiting
code I have no way to test.

> 
> > +       if (ret)
> > +               return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> > +
> > +       /*
> > +        * Get the DSI controller that is supplying data for this display
> > +        * which is controlled via SPI 3-wire.
> Same as above
> 
> > +        */
> > +       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > +       if (!endpoint) {
> > +               dev_err(dev, "failed to get endpoint\n");
> > +               return -ENODEV;
> > +       }
> > +       dsi_host_node = of_graph_get_remote_port_parent(endpoint);
> > +       if (!dsi_host_node) {
> > +               dev_err(dev, "failed to get remote port parent\n");
> > +               goto put_endpoint;
> > +       }
> > +       dsi_host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> > +       if (!dsi_host) {
> > +               dev_err(dev, "failed to find dsi host\n");
> > +               goto put_host;
> > +       }
> > +       info.node = of_graph_get_remote_port(endpoint);
> > +       if (!info.node) {
> > +               dev_err(dev, "failed to get remote port node\n");
> > +               ret = -ENODEV;
> > +               goto put_host;
> > +       }
> > +
> > +       db->dsi_dev = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
> > +       if (IS_ERR(db->dsi_dev)) {
> > +               dev_err(dev, "failed to register dsi device: %ld\n",
> > +                       PTR_ERR(db->dsi_dev));
> > +               ret = PTR_ERR(db->dsi_dev);
> > +               goto put_host;
> > +       }
> > +
> > +       db->dsi_dev->lanes = 2;
> > +       db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
> > +       db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > +                         MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> > +
> > +       drm_panel_init(&db->panel, dev, &ams495qa01_drm_funcs,
> > +                      DRM_MODE_CONNECTOR_DSI);
> > +
> > +       ret = ams495qa01_backlight_register(db);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       drm_panel_add(&db->panel);
> > +
> > +       ret = devm_mipi_dsi_attach(dev, db->dsi_dev);
> > +       if (ret < 0) {
> > +               dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
> > +               drm_panel_remove(&db->panel);
> > +               return ret;
> > +       }
> > +
> > +       of_node_put(dsi_host_node);
> > +       of_node_put(endpoint);
> > +
> > +       return 0;
> > +
> > +put_host:
> > +       of_node_put(dsi_host_node);
> > +
> > +put_endpoint:
> > +       of_node_put(endpoint);
> > +       return -ENODEV;
> > +}
> > +
> > +static void ams495qa01_shutdown(struct spi_device *spi)
> > +{
> > +       struct ams495qa01 *db = spi_get_drvdata(spi);
> > +       int ret;
> > +
> > +       ret = drm_panel_unprepare(&db->panel);
> > +       if (ret < 0)
> > +               dev_err(&spi->dev, "Failed to unprepare panel: %d\n", ret);
> > +
> > +       ret = drm_panel_disable(&db->panel);
> > +       if (ret < 0)
> > +               dev_err(&spi->dev, "Failed to disable panel: %d\n", ret);
> > +}
> > +
> > +static void ams495qa01_remove(struct spi_device *spi)
> > +{
> > +       struct ams495qa01 *db = spi_get_drvdata(spi);
> > +
> > +       ams495qa01_shutdown(spi);
> > +
> > +       drm_panel_remove(&db->panel);
> > +}
> > +
> > +static const struct of_device_id ams495qa01_match[] = {
> > +       { .compatible = "samsung,ams495qa01", },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ams495qa01_match);
> > +
> > +static const struct spi_device_id ams495qa01_ids[] = {
> > +       { "ams495qa01", 0 },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(spi, ams495qa01_ids);
> > +
> > +static struct spi_driver ams495qa01_driver = {
> > +       .driver         = {
> > +               .name   = "ams495qa01-panel",
> > +               .of_match_table = ams495qa01_match,
> > +       },
> > +       .id_table       = ams495qa01_ids,
> > +       .probe          = ams495qa01_probe,
> > +       .remove         = ams495qa01_remove,
> > +       .shutdown       = ams495qa01_shutdown,
> > +};
> > +module_spi_driver(ams495qa01_driver);
> > +
> > +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> > +MODULE_DESCRIPTION("Samsung ams495qa01 panel driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
> 
> Best Regards,
> Maya Matuszczyk

As always appreciate the feedback and enjoy working with you.

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

* Re: [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
  2022-09-20 17:09   ` Chris Morgan
@ 2022-09-21  7:23     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-21  7:23 UTC (permalink / raw)
  To: Chris Morgan, dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, robh+dt, daniel, airlied,
	sam, thierry.reding, Chris Morgan

On 20/09/2022 19:09, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add documentation for the Samsung AMS495QA01 panel.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../display/panel/samsung,ams495qa01.yaml     | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> new file mode 100644
> index 000000000000..08358cdad19c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung AMS495QA01 4.95in 960x544 DSI/SPI panel
> +
> +maintainers:
> +  - Chris Morgan <macromorgan@hotmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: samsung,ams495qa01

Blank line.


> +  reg: true
> +  reset-gpios: true
> +  elvdd-supply:
> +    description: regulator that supplies voltage to the panel display
> +  enable-gpios: true
> +  port: true
> +  vdd-supply:
> +    description: regulator that supplies voltage to panel logic


Best regards,
Krzysztof

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

* Re: [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
@ 2022-09-21  7:23     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-21  7:23 UTC (permalink / raw)
  To: Chris Morgan, dri-devel
  Cc: devicetree, krzysztof.kozlowski+dt, airlied, Chris Morgan,
	robh+dt, thierry.reding, sam

On 20/09/2022 19:09, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add documentation for the Samsung AMS495QA01 panel.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../display/panel/samsung,ams495qa01.yaml     | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> new file mode 100644
> index 000000000000..08358cdad19c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,ams495qa01.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung AMS495QA01 4.95in 960x544 DSI/SPI panel
> +
> +maintainers:
> +  - Chris Morgan <macromorgan@hotmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: samsung,ams495qa01

Blank line.


> +  reg: true
> +  reset-gpios: true
> +  elvdd-supply:
> +    description: regulator that supplies voltage to the panel display
> +  enable-gpios: true
> +  port: true
> +  vdd-supply:
> +    description: regulator that supplies voltage to panel logic


Best regards,
Krzysztof

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

end of thread, other threads:[~2022-09-21  7:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20 17:09 [PATCH V2 0/2] drm/panel: Add Samsung AMS495QA01 Panel Chris Morgan
2022-09-20 17:09 ` Chris Morgan
2022-09-20 17:09 ` [PATCH V2 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings Chris Morgan
2022-09-20 17:09   ` Chris Morgan
2022-09-20 17:38   ` Maya Matuszczyk
2022-09-20 17:38     ` Maya Matuszczyk
2022-09-21  7:23   ` Krzysztof Kozlowski
2022-09-21  7:23     ` Krzysztof Kozlowski
2022-09-20 17:09 ` [PATCH V2 2/2] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel Chris Morgan
2022-09-20 17:09   ` Chris Morgan
2022-09-20 17:33   ` Maya Matuszczyk
2022-09-20 17:33     ` Maya Matuszczyk
2022-09-20 21:03     ` Chris Morgan
2022-09-20 21:03       ` Chris Morgan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.