All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
@ 2021-08-03 13:54 Markuss Broks
  2021-08-03 14:45 ` Sam Ravnborg
  0 siblings, 1 reply; 21+ messages in thread
From: Markuss Broks @ 2021-08-03 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: linus.walleij, phone-devel, Markuss Broks, Thierry Reding,
	Sam Ravnborg, David Airlie, Daniel Vetter, dri-devel

This adds a driver for Samsung S6D27A1 display controller and panel.
This panel is found in the Samsung GT-I8160 mobile phone,
and possibly some other mobile phones.

This display needs manufactrer commands to configure it to a working state;
the commands used in this driver were taken from downstream driver
by Gareth Phillips; sadly, there is almost no documentation on what they
actually do.

This driver re-uses the DBI infrastructure to communicate with the display.

This driver is heavily based on WideChips WS2401 display controller
driver by Linus Walleij and on other panel drivers for reference.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |   8 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 342 ++++++++++++++++++
 3 files changed, 351 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index f4fe1dba99..21007b2032 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -348,6 +348,14 @@ config DRM_PANEL_SAMSUNG_S6D16D0
 	depends on DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
+config DRM_PANEL_SAMSUNG_S6D27A1
+	tristate "Samsung S6D27A1 DPI panel driver"
+	depends on OF && SPI && GPIOLIB
+	select DRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for the Samsung
+	  S6D27A1 DPI 480x800 panel.
+
 config DRM_PANEL_SAMSUNG_S6E3HA2
 	tristate "Samsung S6E3HA2 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d94c27df17..d2a6fe81e9 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -35,6 +35,7 @@ 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_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
new file mode 100644
index 0000000000..86cf8d316b
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Panel driver for the Samsung S6D27A1 480x800 DPI RGB panel.
+ * Found in the Samsung Galaxy Ace 2 GT-I8160 mobile phone.
+ */
+
+#include <drm/drm_modes.h>
+#include <drm/drm_mipi_dbi.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/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+#define S6D27A1_PASSWD_L2	0xF0	/* Password Command for Level 2 Control */
+#define S6D27A1_RESCTL		0xB3	/* Resolution Select Control */
+#define S6D27A1_PANELCTL2	0xB4	/* ASG Signal Control */
+#define S6D27A1_READID1		0xDA	/* Read panel ID 1 */
+#define S6D27A1_READID2		0xDB	/* Read panel ID 2 */
+#define S6D27A1_READID3		0xDC	/* Read panel ID 3 */
+#define S6D27A1_DISPCTL		0xF2	/* Display Control */
+#define S6D27A1_MANPWR		0xF3	/* Manual Control */
+#define S6D27A1_PWRCTL1		0xF4	/* Power Control */
+#define S6D27A1_SRCCTL		0xF6	/* Source Control */
+#define S6D27A1_PANELCTL	0xF7	/* Panel Control*/
+
+static const u8 s6d27a1_dbi_read_commands[] = {
+	S6D27A1_READID1,
+	S6D27A1_READID2,
+	S6D27A1_READID3,
+	0, /* sentinel */
+};
+
+/**
+ * struct s6d27a1 - state container for a panel controlled by the WS2401
+ * controller
+ */
+struct s6d27a1 {
+	/** @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;
+	/** @width: the width of this panel in mm */
+	u32 width;
+	/** @height: the height of this panel in mm */
+	u32 height;
+	/** @reset: reset GPIO line */
+	struct gpio_desc *reset;
+	/** @regulators: VCCIO and VIO supply regulators */
+	struct regulator_bulk_data regulators[2];
+	/** @bl: Backlight, if using internal backlight */
+	struct backlight_device *bl;
+};
+
+static const struct drm_display_mode s6d27a1_480_800_mode = {
+	/*
+	 * The vendor driver states that the S6D27A1 panel
+	 * has a pixel clock frequency of 49920000 Hz / 2 = 24960000 Hz.
+	 */
+	.clock = 24960,
+	.hdisplay = 480,
+	.hsync_start = 480 + 63,
+	.hsync_end = 480 + 63 + 2,
+	.htotal = 480 + 63 + 2 + 63,
+	.vdisplay = 800,
+	.vsync_start = 800 + 11,
+	.vsync_end = 800 + 11 + 2,
+	.vtotal = 800 + 11 + 2 + 10,
+	.width_mm = 50,
+	.height_mm = 84,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static inline struct s6d27a1 *to_s6d27a1(struct drm_panel *panel)
+{
+	return container_of(panel, struct s6d27a1, panel);
+}
+
+#define s6d27a1_command(ctx, cmd, seq...) \
+({ \
+	struct mipi_dbi *dbi = &ctx->dbi; \
+	int ret; \
+	ret = mipi_dbi_command(dbi, cmd, seq);	\
+	if (ret) { \
+		dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
+	} \
+})
+
+static void s6d27a1_read_mtp_id(struct s6d27a1 *ctx)
+{
+	struct mipi_dbi *dbi = &ctx->dbi;
+	u8 id1, id2, id3;
+	int ret;
+
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID1, &id1);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 1\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID2, &id1);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 2\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID3, &id1);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 3\n");
+		return;
+	}
+	dev_info(ctx->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
+}
+
+static int s6d27a1_power_on(struct s6d27a1 *ctx)
+{
+	int ret;
+
+	/* Power up */
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->regulators),
+				    ctx->regulators);
+	if (ret) {
+		dev_err(ctx->dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+	msleep(20);
+
+	/* Assert reset >=1 ms */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	usleep_range(1000, 5000);
+	/* De-assert reset */
+	gpiod_set_value_cansleep(ctx->reset, 0);
+	/* Wait >= 10 ms */
+	msleep(20);
+	dev_dbg(ctx->dev, "de-asserted RESET\n");
+
+	/*
+	 * Exit sleep mode and initialize display - some hammering is
+	 * necessary.
+	 */
+	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(50);
+
+	/* Magic to unlock level 2 control of the display */
+	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0x5A, 0x5A);
+
+	/* Configure resolution to 480RGBx800 */
+	s6d27a1_command(ctx, S6D27A1_RESCTL, 0x22);
+
+	s6d27a1_command(ctx, S6D27A1_PANELCTL2, 0x00, 0x02, 0x03, 0x04, 0x05, 0x08, 0x00, 0x0c);
+
+	s6d27a1_command(ctx, S6D27A1_MANPWR, 0x01, 0x00, 0x00, 0x08, 0x08, 0x02, 0x00);
+
+	s6d27a1_command(ctx, S6D27A1_DISPCTL, 0x19, 0x00, 0x08, 0x0D, 0x03, 0x41, 0x3F);
+
+	s6d27a1_command(ctx, S6D27A1_PWRCTL1, 0x00, 0x00, 0x00, 0x00, 0x55,
+						0x44, 0x05, 0x88, 0x4B, 0x50);
+
+	s6d27a1_command(ctx, S6D27A1_SRCCTL, 0x03, 0x09, 0x8A, 0x00, 0x01, 0x16);
+
+	s6d27a1_command(ctx, S6D27A1_PANELCTL, 0x00, 0x05, 0x06, 0x07, 0x08,
+						0x01, 0x09, 0x0D, 0x0A, 0x0E,
+						0x0B, 0x0F, 0x0C, 0x10, 0x01,
+						0x11, 0x12, 0x13, 0x14, 0x05,
+						0x06, 0x07, 0x08, 0x01, 0x09,
+						0x0D, 0x0A, 0x0E, 0x0B, 0x0F,
+						0x0C, 0x10, 0x01, 0x11, 0x12,
+						0x13, 0x14);
+
+	/* lock the level 2 control */
+	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0xA5, 0xA5);
+
+	s6d27a1_read_mtp_id(ctx);
+
+	return 0;
+}
+
+static int s6d27a1_power_off(struct s6d27a1 *ctx)
+{
+	/* Go into RESET and disable regulators */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+}
+
+static int s6d27a1_unprepare(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(120);
+	return s6d27a1_power_off(to_s6d27a1(panel));
+}
+
+static int s6d27a1_disable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(25);
+
+	return 0;
+}
+
+static int s6d27a1_prepare(struct drm_panel *panel)
+{
+	return s6d27a1_power_on(to_s6d27a1(panel));
+}
+
+static int s6d27a1_enable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_ON);
+
+	return 0;
+}
+
+/**
+ * s6d27a1_get_modes() - return the mode
+ * @panel: the panel to get the mode for
+ * @connector: reference to the central DRM connector control structure
+ */
+static int s6d27a1_get_modes(struct drm_panel *panel,
+			    struct drm_connector *connector)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, &s6d27a1_480_800_mode);
+	if (!mode) {
+		dev_err(ctx->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_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 s6d27a1_drm_funcs = {
+	.disable = s6d27a1_disable,
+	.unprepare = s6d27a1_unprepare,
+	.prepare = s6d27a1_prepare,
+	.enable = s6d27a1_enable,
+	.get_modes = s6d27a1_get_modes,
+};
+
+static int s6d27a1_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct s6d27a1 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	ctx->dev = dev;
+
+	/*
+	 * VCI   is the analog voltage supply
+	 * VCCIO is the digital I/O voltage supply
+	 */
+	ctx->regulators[0].supply = "vci";
+	ctx->regulators[1].supply = "vccio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	ctx->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset)) {
+		ret = PTR_ERR(ctx->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &ctx->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+	ctx->dbi.read_commands = s6d27a1_dbi_read_commands;
+
+	drm_panel_init(&ctx->panel, dev, &s6d27a1_drm_funcs,
+		       DRM_MODE_CONNECTOR_DPI);
+
+	spi_set_drvdata(spi, ctx);
+
+	drm_panel_add(&ctx->panel);
+	dev_dbg(dev, "added panel\n");
+
+	return 0;
+}
+
+static int s6d27a1_remove(struct spi_device *spi)
+{
+	struct s6d27a1 *ctx = spi_get_drvdata(spi);
+
+	drm_panel_remove(&ctx->panel);
+	return 0;
+}
+
+static const struct of_device_id s6d27a1_match[] = {
+	{ .compatible = "samsung,s6d27a1", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, s6d27a1_match);
+
+static struct spi_driver s6d27a1_driver = {
+	.probe		= s6d27a1_probe,
+	.remove		= s6d27a1_remove,
+	.driver		= {
+		.name	= "s6d27a1-panel",
+		.of_match_table = s6d27a1_match,
+	},
+};
+module_spi_driver(s6d27a1_driver);
+
+MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>");
+MODULE_DESCRIPTION("Samsung S6D27A1 panel driver");
+MODULE_LICENSE("GPL v2");
-- 
2.32.0


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

* Re: [PATCH] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-03 13:54 [PATCH] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel Markuss Broks
@ 2021-08-03 14:45 ` Sam Ravnborg
  2021-08-03 17:24   ` [PATCH v2] " Markuss Broks
  0 siblings, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2021-08-03 14:45 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, linus.walleij, phone-devel, Thierry Reding,
	David Airlie, Daniel Vetter, dri-devel

Hi Markuss,
just a random collection of minor things to fix.
Overall the driver looks good.

	Sam

On Tue, Aug 03, 2021 at 04:54:18PM +0300, Markuss Broks wrote:
> This adds a driver for Samsung S6D27A1 display controller and panel.
> This panel is found in the Samsung GT-I8160 mobile phone,
> and possibly some other mobile phones.
> 
> This display needs manufactrer commands to configure it to a working state;
fix spelling of manufactrer

> the commands used in this driver were taken from downstream driver
> by Gareth Phillips; sadly, there is almost no documentation on what they
> actually do.
> 
> This driver re-uses the DBI infrastructure to communicate with the display.
> 
> This driver is heavily based on WideChips WS2401 display controller
> driver by Linus Walleij and on other panel drivers for reference.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   8 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 342 ++++++++++++++++++
>  3 files changed, 351 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index f4fe1dba99..21007b2032 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -348,6 +348,14 @@ config DRM_PANEL_SAMSUNG_S6D16D0
>  	depends on DRM_MIPI_DSI
>  	select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_SAMSUNG_S6D27A1
> +	tristate "Samsung S6D27A1 DPI panel driver"
> +	depends on OF && SPI && GPIOLIB
> +	select DRM_MIPI_DBI
> +	help
> +	  Say Y here if you want to enable support for the Samsung
> +	  S6D27A1 DPI 480x800 panel.
> +
>  config DRM_PANEL_SAMSUNG_S6E3HA2
>  	tristate "Samsung S6E3HA2 DSI video mode panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index d94c27df17..d2a6fe81e9 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -35,6 +35,7 @@ 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_LD9040) += panel-samsung-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
> new file mode 100644
> index 0000000000..86cf8d316b
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Panel driver for the Samsung S6D27A1 480x800 DPI RGB panel.
> + * Found in the Samsung Galaxy Ace 2 GT-I8160 mobile phone.
> + */
> +
> +#include <drm/drm_modes.h>
> +#include <drm/drm_mipi_dbi.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/backlight.h>
No backlight support - drop header.

> +#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/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <video/mipi_display.h>
> +
> +#define S6D27A1_PASSWD_L2	0xF0	/* Password Command for Level 2 Control */
> +#define S6D27A1_RESCTL		0xB3	/* Resolution Select Control */
> +#define S6D27A1_PANELCTL2	0xB4	/* ASG Signal Control */
> +#define S6D27A1_READID1		0xDA	/* Read panel ID 1 */
> +#define S6D27A1_READID2		0xDB	/* Read panel ID 2 */
> +#define S6D27A1_READID3		0xDC	/* Read panel ID 3 */
> +#define S6D27A1_DISPCTL		0xF2	/* Display Control */
> +#define S6D27A1_MANPWR		0xF3	/* Manual Control */
> +#define S6D27A1_PWRCTL1		0xF4	/* Power Control */
> +#define S6D27A1_SRCCTL		0xF6	/* Source Control */
> +#define S6D27A1_PANELCTL	0xF7	/* Panel Control*/
> +
> +static const u8 s6d27a1_dbi_read_commands[] = {
> +	S6D27A1_READID1,
> +	S6D27A1_READID2,
> +	S6D27A1_READID3,
> +	0, /* sentinel */
> +};
> +
> +/**
> + * struct s6d27a1 - state container for a panel controlled by the WS2401
> + * controller
> + */
I see no value in kernel-doc style here.

> +struct s6d27a1 {
> +	/** @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;
> +	/** @width: the width of this panel in mm */
> +	u32 width;
Drop, not used.

> +	/** @height: the height of this panel in mm */
> +	u32 height;
Drop, not used.

> +	/** @reset: reset GPIO line */
> +	struct gpio_desc *reset;
> +	/** @regulators: VCCIO and VIO supply regulators */
> +	struct regulator_bulk_data regulators[2];
Names in comments does not match the code.

> +	/** @bl: Backlight, if using internal backlight */
> +	struct backlight_device *bl;
Drop, not used.
> +};
> +
> +static const struct drm_display_mode s6d27a1_480_800_mode = {
> +	/*
> +	 * The vendor driver states that the S6D27A1 panel
> +	 * has a pixel clock frequency of 49920000 Hz / 2 = 24960000 Hz.
> +	 */
> +	.clock = 24960,
> +	.hdisplay = 480,
> +	.hsync_start = 480 + 63,
> +	.hsync_end = 480 + 63 + 2,
> +	.htotal = 480 + 63 + 2 + 63,
> +	.vdisplay = 800,
> +	.vsync_start = 800 + 11,
> +	.vsync_end = 800 + 11 + 2,
> +	.vtotal = 800 + 11 + 2 + 10,
> +	.width_mm = 50,
> +	.height_mm = 84,
> +	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +};
> +
> +static inline struct s6d27a1 *to_s6d27a1(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct s6d27a1, panel);
> +}
> +
> +#define s6d27a1_command(ctx, cmd, seq...) \
> +({ \
> +	struct mipi_dbi *dbi = &ctx->dbi; \
> +	int ret; \
> +	ret = mipi_dbi_command(dbi, cmd, seq);	\
> +	if (ret) { \
> +		dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
> +	} \
> +})
> +
> +static void s6d27a1_read_mtp_id(struct s6d27a1 *ctx)
> +{
> +	struct mipi_dbi *dbi = &ctx->dbi;
> +	u8 id1, id2, id3;
> +	int ret;
> +
> +	ret = mipi_dbi_command_read(dbi, S6D27A1_READID1, &id1);
> +	if (ret) {
> +		dev_err(ctx->dev, "unable to read MTP ID 1\n");
> +		return;
> +	}
> +	ret = mipi_dbi_command_read(dbi, S6D27A1_READID2, &id1);
Here you wanted to use id2

> +	if (ret) {
> +		dev_err(ctx->dev, "unable to read MTP ID 2\n");
> +		return;
> +	}
> +	ret = mipi_dbi_command_read(dbi, S6D27A1_READID3, &id1);
Here you wanted to use id3

> +	if (ret) {
> +		dev_err(ctx->dev, "unable to read MTP ID 3\n");
> +		return;
> +	}
> +	dev_info(ctx->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
Please verify that this prints the correct value to verify the above
code.

> +}
> +
> +static int s6d27a1_power_on(struct s6d27a1 *ctx)
> +{
> +	int ret;
> +
> +	/* Power up */
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->regulators),
> +				    ctx->regulators);
> +	if (ret) {
> +		dev_err(ctx->dev, "failed to enable regulators: %d\n", ret);
> +		return ret;
> +	}
> +	msleep(20);
Add empty line after closing '}'

> +
> +	/* Assert reset >=1 ms */
> +	gpiod_set_value_cansleep(ctx->reset, 1);
> +	usleep_range(1000, 5000);
> +	/* De-assert reset */
> +	gpiod_set_value_cansleep(ctx->reset, 0);
> +	/* Wait >= 10 ms */
> +	msleep(20);

> +	dev_dbg(ctx->dev, "de-asserted RESET\n");
Looks like some debug leftover that can be dropped.

> +
> +	/*
> +	 * Exit sleep mode and initialize display - some hammering is
> +	 * necessary.
> +	 */
> +	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
> +	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
> +	msleep(50);
> +
> +	/* Magic to unlock level 2 control of the display */
> +	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0x5A, 0x5A);
> +
> +	/* Configure resolution to 480RGBx800 */
> +	s6d27a1_command(ctx, S6D27A1_RESCTL, 0x22);
> +
> +	s6d27a1_command(ctx, S6D27A1_PANELCTL2, 0x00, 0x02, 0x03, 0x04, 0x05, 0x08, 0x00, 0x0c);
> +
> +	s6d27a1_command(ctx, S6D27A1_MANPWR, 0x01, 0x00, 0x00, 0x08, 0x08, 0x02, 0x00);
> +
> +	s6d27a1_command(ctx, S6D27A1_DISPCTL, 0x19, 0x00, 0x08, 0x0D, 0x03, 0x41, 0x3F);
> +
> +	s6d27a1_command(ctx, S6D27A1_PWRCTL1, 0x00, 0x00, 0x00, 0x00, 0x55,
> +						0x44, 0x05, 0x88, 0x4B, 0x50);
> +
> +	s6d27a1_command(ctx, S6D27A1_SRCCTL, 0x03, 0x09, 0x8A, 0x00, 0x01, 0x16);
> +
> +	s6d27a1_command(ctx, S6D27A1_PANELCTL, 0x00, 0x05, 0x06, 0x07, 0x08,
> +						0x01, 0x09, 0x0D, 0x0A, 0x0E,
> +						0x0B, 0x0F, 0x0C, 0x10, 0x01,
> +						0x11, 0x12, 0x13, 0x14, 0x05,
> +						0x06, 0x07, 0x08, 0x01, 0x09,
> +						0x0D, 0x0A, 0x0E, 0x0B, 0x0F,
> +						0x0C, 0x10, 0x01, 0x11, 0x12,
> +						0x13, 0x14);
> +
> +	/* lock the level 2 control */
> +	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0xA5, 0xA5);
> +
> +	s6d27a1_read_mtp_id(ctx);
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_power_off(struct s6d27a1 *ctx)
> +{
> +	/* Go into RESET and disable regulators */
> +	gpiod_set_value_cansleep(ctx->reset, 1);
> +	return regulator_bulk_disable(ARRAY_SIZE(ctx->regulators),
> +				      ctx->regulators);
> +}
> +
> +static int s6d27a1_unprepare(struct drm_panel *panel)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +
> +	s6d27a1_command(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
> +	msleep(120);
> +	return s6d27a1_power_off(to_s6d27a1(panel));
> +}
> +
> +static int s6d27a1_disable(struct drm_panel *panel)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +
> +	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_OFF);
> +	msleep(25);
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_prepare(struct drm_panel *panel)
> +{
> +	return s6d27a1_power_on(to_s6d27a1(panel));
> +}
> +
> +static int s6d27a1_enable(struct drm_panel *panel)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +
> +	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_ON);
> +
> +	return 0;
> +}
> +
> +/**
> + * s6d27a1_get_modes() - return the mode
> + * @panel: the panel to get the mode for
> + * @connector: reference to the central DRM connector control structure
> + */
Kernel-doc here seems irrelevant. And drop the descriptions of
parameters too.
> +static int s6d27a1_get_modes(struct drm_panel *panel,
> +			    struct drm_connector *connector)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +	struct drm_display_mode *mode;
> +	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +
> +	mode = drm_mode_duplicate(connector->dev, &s6d27a1_480_800_mode);
> +	if (!mode) {
> +		dev_err(ctx->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_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 s6d27a1_drm_funcs = {
> +	.disable = s6d27a1_disable,
> +	.unprepare = s6d27a1_unprepare,
> +	.prepare = s6d27a1_prepare,
> +	.enable = s6d27a1_enable,
> +	.get_modes = s6d27a1_get_modes,
> +};
> +
> +static int s6d27a1_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct s6d27a1 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
Add empty line here
> +	ctx->dev = dev;
> +
> +	/*
> +	 * VCI   is the analog voltage supply
> +	 * VCCIO is the digital I/O voltage supply
> +	 */
> +	ctx->regulators[0].supply = "vci";
> +	ctx->regulators[1].supply = "vccio";
> +	ret = devm_regulator_bulk_get(dev,
> +				      ARRAY_SIZE(ctx->regulators),
> +				      ctx->regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	ctx->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->reset)) {
> +		ret = PTR_ERR(ctx->reset);
> +		return dev_err_probe(dev, ret, "no RESET GPIO\n");
> +	}
> +
> +	ret = mipi_dbi_spi_init(spi, &ctx->dbi, NULL);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
Add empty line here
> +	ctx->dbi.read_commands = s6d27a1_dbi_read_commands;
> +
> +	drm_panel_init(&ctx->panel, dev, &s6d27a1_drm_funcs,
> +		       DRM_MODE_CONNECTOR_DPI);
> +
> +	spi_set_drvdata(spi, ctx);
> +
> +	drm_panel_add(&ctx->panel);
> +	dev_dbg(dev, "added panel\n");
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_remove(struct spi_device *spi)
> +{
> +	struct s6d27a1 *ctx = spi_get_drvdata(spi);
> +
> +	drm_panel_remove(&ctx->panel);
> +	return 0;
> +}
> +
> +static const struct of_device_id s6d27a1_match[] = {
> +	{ .compatible = "samsung,s6d27a1", },
> +	{},
Write as
	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, s6d27a1_match);
> +
> +static struct spi_driver s6d27a1_driver = {
> +	.probe		= s6d27a1_probe,
> +	.remove		= s6d27a1_remove,
> +	.driver		= {
> +		.name	= "s6d27a1-panel",
> +		.of_match_table = s6d27a1_match,
> +	},
> +};
> +module_spi_driver(s6d27a1_driver);
> +
> +MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>");
> +MODULE_DESCRIPTION("Samsung S6D27A1 panel driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.32.0

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

* [PATCH v2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-03 14:45 ` Sam Ravnborg
@ 2021-08-03 17:24   ` Markuss Broks
  2021-08-03 18:11     ` Sam Ravnborg
  0 siblings, 1 reply; 21+ messages in thread
From: Markuss Broks @ 2021-08-03 17:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: linus.walleij, phone-devel, Markuss Broks, Thierry Reding,
	Sam Ravnborg, David Airlie, Daniel Vetter, dri-devel

This adds a driver for Samsung S6D27A1 display controller and panel.
This panel is found in the Samsung GT-I8160 mobile phone,
and possibly some other mobile phones.

This display needs manufacturer commands to configure it to a working state;
the commands used in this driver were taken from downstream driver
by Gareth Phillips; sadly, there is almost no documentation on what they
actually do.

This driver re-uses the DBI infrastructure to communicate with the display.

This driver is heavily based on WideChips WS2401 display controller
driver by Linus Walleij and on other panel drivers for reference.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |   8 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 323 ++++++++++++++++++
 3 files changed, 332 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index f4fe1dba99..21007b2032 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -348,6 +348,14 @@ config DRM_PANEL_SAMSUNG_S6D16D0
 	depends on DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
+config DRM_PANEL_SAMSUNG_S6D27A1
+	tristate "Samsung S6D27A1 DPI panel driver"
+	depends on OF && SPI && GPIOLIB
+	select DRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for the Samsung
+	  S6D27A1 DPI 480x800 panel.
+
 config DRM_PANEL_SAMSUNG_S6E3HA2
 	tristate "Samsung S6E3HA2 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d94c27df17..d2a6fe81e9 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -35,6 +35,7 @@ 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_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
new file mode 100644
index 0000000000..073454349d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Panel driver for the Samsung S6D27A1 480x800 DPI RGB panel.
+ * Found in the Samsung Galaxy Ace 2 GT-I8160 mobile phone.
+ */
+
+#include <drm/drm_modes.h>
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_panel.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/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+#define S6D27A1_PASSWD_L2	0xF0	/* Password Command for Level 2 Control */
+#define S6D27A1_RESCTL		0xB3	/* Resolution Select Control */
+#define S6D27A1_PANELCTL2	0xB4	/* ASG Signal Control */
+#define S6D27A1_READID1		0xDA	/* Read panel ID 1 */
+#define S6D27A1_READID2		0xDB	/* Read panel ID 2 */
+#define S6D27A1_READID3		0xDC	/* Read panel ID 3 */
+#define S6D27A1_DISPCTL		0xF2	/* Display Control */
+#define S6D27A1_MANPWR		0xF3	/* Manual Control */
+#define S6D27A1_PWRCTL1		0xF4	/* Power Control */
+#define S6D27A1_SRCCTL		0xF6	/* Source Control */
+#define S6D27A1_PANELCTL	0xF7	/* Panel Control*/
+
+static const u8 s6d27a1_dbi_read_commands[] = {
+	S6D27A1_READID1,
+	S6D27A1_READID2,
+	S6D27A1_READID3,
+	0, /* sentinel */
+};
+
+struct s6d27a1 {
+	struct device *dev;
+	struct mipi_dbi dbi;
+	struct drm_panel panel;
+	struct gpio_desc *reset;
+	struct regulator_bulk_data regulators[2];
+};
+
+static const struct drm_display_mode s6d27a1_480_800_mode = {
+	/*
+	 * The vendor driver states that the S6D27A1 panel
+	 * has a pixel clock frequency of 49920000 Hz / 2 = 24960000 Hz.
+	 */
+	.clock = 24960,
+	.hdisplay = 480,
+	.hsync_start = 480 + 63,
+	.hsync_end = 480 + 63 + 2,
+	.htotal = 480 + 63 + 2 + 63,
+	.vdisplay = 800,
+	.vsync_start = 800 + 11,
+	.vsync_end = 800 + 11 + 2,
+	.vtotal = 800 + 11 + 2 + 10,
+	.width_mm = 50,
+	.height_mm = 84,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static inline struct s6d27a1 *to_s6d27a1(struct drm_panel *panel)
+{
+	return container_of(panel, struct s6d27a1, panel);
+}
+
+#define s6d27a1_command(ctx, cmd, seq...) \
+({ \
+	struct mipi_dbi *dbi = &ctx->dbi; \
+	int ret; \
+	ret = mipi_dbi_command(dbi, cmd, seq);	\
+	if (ret) { \
+		dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
+	} \
+})
+
+static void s6d27a1_read_mtp_id(struct s6d27a1 *ctx)
+{
+	struct mipi_dbi *dbi = &ctx->dbi;
+	u8 id1, id2, id3;
+	int ret;
+
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID1, &id1);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 1\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID2, &id2);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 2\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID3, &id3);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 3\n");
+		return;
+	}
+	dev_info(ctx->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
+}
+
+static int s6d27a1_power_on(struct s6d27a1 *ctx)
+{
+	int ret;
+
+	/* Power up */
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->regulators),
+				    ctx->regulators);
+	if (ret) {
+		dev_err(ctx->dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	msleep(20);
+
+	/* Assert reset >=1 ms */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	usleep_range(1000, 5000);
+	/* De-assert reset */
+	gpiod_set_value_cansleep(ctx->reset, 0);
+	/* Wait >= 10 ms */
+	msleep(20);
+
+	/*
+	 * Exit sleep mode and initialize display - some hammering is
+	 * necessary.
+	 */
+	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(50);
+
+	/* Magic to unlock level 2 control of the display */
+	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0x5A, 0x5A);
+
+	/* Configure resolution to 480RGBx800 */
+	s6d27a1_command(ctx, S6D27A1_RESCTL, 0x22);
+
+	s6d27a1_command(ctx, S6D27A1_PANELCTL2, 0x00, 0x02, 0x03, 0x04, 0x05, 0x08, 0x00, 0x0c);
+
+	s6d27a1_command(ctx, S6D27A1_MANPWR, 0x01, 0x00, 0x00, 0x08, 0x08, 0x02, 0x00);
+
+	s6d27a1_command(ctx, S6D27A1_DISPCTL, 0x19, 0x00, 0x08, 0x0D, 0x03, 0x41, 0x3F);
+
+	s6d27a1_command(ctx, S6D27A1_PWRCTL1, 0x00, 0x00, 0x00, 0x00, 0x55,
+						0x44, 0x05, 0x88, 0x4B, 0x50);
+
+	s6d27a1_command(ctx, S6D27A1_SRCCTL, 0x03, 0x09, 0x8A, 0x00, 0x01, 0x16);
+
+	s6d27a1_command(ctx, S6D27A1_PANELCTL, 0x00, 0x05, 0x06, 0x07, 0x08,
+						0x01, 0x09, 0x0D, 0x0A, 0x0E,
+						0x0B, 0x0F, 0x0C, 0x10, 0x01,
+						0x11, 0x12, 0x13, 0x14, 0x05,
+						0x06, 0x07, 0x08, 0x01, 0x09,
+						0x0D, 0x0A, 0x0E, 0x0B, 0x0F,
+						0x0C, 0x10, 0x01, 0x11, 0x12,
+						0x13, 0x14);
+
+	/* lock the level 2 control */
+	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0xA5, 0xA5);
+
+	s6d27a1_read_mtp_id(ctx);
+
+	return 0;
+}
+
+static int s6d27a1_power_off(struct s6d27a1 *ctx)
+{
+	/* Go into RESET and disable regulators */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+}
+
+static int s6d27a1_unprepare(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(120);
+	return s6d27a1_power_off(to_s6d27a1(panel));
+}
+
+static int s6d27a1_disable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(25);
+
+	return 0;
+}
+
+static int s6d27a1_prepare(struct drm_panel *panel)
+{
+	return s6d27a1_power_on(to_s6d27a1(panel));
+}
+
+static int s6d27a1_enable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_ON);
+
+	return 0;
+}
+
+static int s6d27a1_get_modes(struct drm_panel *panel,
+			    struct drm_connector *connector)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, &s6d27a1_480_800_mode);
+	if (!mode) {
+		dev_err(ctx->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_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 s6d27a1_drm_funcs = {
+	.disable = s6d27a1_disable,
+	.unprepare = s6d27a1_unprepare,
+	.prepare = s6d27a1_prepare,
+	.enable = s6d27a1_enable,
+	.get_modes = s6d27a1_get_modes,
+};
+
+static int s6d27a1_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct s6d27a1 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->dev = dev;
+
+	/*
+	 * VCI   is the analog voltage supply
+	 * VCCIO is the digital I/O voltage supply
+	 */
+	ctx->regulators[0].supply = "vci";
+	ctx->regulators[1].supply = "vccio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	ctx->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset)) {
+		ret = PTR_ERR(ctx->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &ctx->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	ctx->dbi.read_commands = s6d27a1_dbi_read_commands;
+
+	drm_panel_init(&ctx->panel, dev, &s6d27a1_drm_funcs,
+		       DRM_MODE_CONNECTOR_DPI);
+
+	spi_set_drvdata(spi, ctx);
+
+	drm_panel_add(&ctx->panel);
+	dev_dbg(dev, "added panel\n");
+
+	return 0;
+}
+
+static int s6d27a1_remove(struct spi_device *spi)
+{
+	struct s6d27a1 *ctx = spi_get_drvdata(spi);
+
+	drm_panel_remove(&ctx->panel);
+	return 0;
+}
+
+static const struct of_device_id s6d27a1_match[] = {
+	{ .compatible = "samsung,s6d27a1", },
+    { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s6d27a1_match);
+
+static struct spi_driver s6d27a1_driver = {
+	.probe		= s6d27a1_probe,
+	.remove		= s6d27a1_remove,
+	.driver		= {
+		.name	= "s6d27a1-panel",
+		.of_match_table = s6d27a1_match,
+	},
+};
+module_spi_driver(s6d27a1_driver);
+
+MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>");
+MODULE_DESCRIPTION("Samsung S6D27A1 panel driver");
+MODULE_LICENSE("GPL v2");
-- 
2.32.0


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

* Re: [PATCH v2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-03 17:24   ` [PATCH v2] " Markuss Broks
@ 2021-08-03 18:11     ` Sam Ravnborg
  2021-08-05 13:33       ` [PATCH 0/2] Add support " Markuss Broks
  0 siblings, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2021-08-03 18:11 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, linus.walleij, phone-devel, Thierry Reding,
	David Airlie, Daniel Vetter, dri-devel

Hi Markuss,
On Tue, Aug 03, 2021 at 08:24:50PM +0300, Markuss Broks wrote:
> This adds a driver for Samsung S6D27A1 display controller and panel.
> This panel is found in the Samsung GT-I8160 mobile phone,
> and possibly some other mobile phones.
> 
> This display needs manufacturer commands to configure it to a working state;
> the commands used in this driver were taken from downstream driver
> by Gareth Phillips; sadly, there is almost no documentation on what they
> actually do.
> 
> This driver re-uses the DBI infrastructure to communicate with the display.
> 
> This driver is heavily based on WideChips WS2401 display controller
> driver by Linus Walleij and on other panel drivers for reference.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>

Thnaks for the very quick response.

Please provide a changelog so we can follow what has changed.
Sometimes we revisit patches week and 100 patches later and cannot
remember any feedback.

Like for example (my style):
v3:
  - fixed checkpatch warnings (sam)
  - ...

v2:
  - dropped backlight (sam)
  - dropped kernel-doc (sam)
  - bla bla (someone)

There are many different styles - pick one.



Also two other things.
checkpatch --strict reports a few issues to be fixed:

    total: 0 errors, 5 warnings, 6 checks, 344 lines checked

One of them is a missing binding!

And the patch fails to apply. Please rebase on top of -next or
drm-misc-next.

Thanks in advance!

	Sam

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

* [PATCH 0/2] Add support for Samsung S6D27A1 display panel
  2021-08-03 18:11     ` Sam Ravnborg
@ 2021-08-05 13:33       ` Markuss Broks
  2021-08-05 13:33         ` [PATCH 1/2] drm/panel: Add DT bindings " Markuss Broks
  2021-08-05 13:33         ` [PATCH 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
  0 siblings, 2 replies; 21+ messages in thread
From: Markuss Broks @ 2021-08-05 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linus.walleij, phone-devel, Markuss Broks, Thierry Reding,
	Sam Ravnborg, David Airlie, Daniel Vetter, Rob Herring,
	dri-devel, devicetree

Add support for Samsung Mobile Displays(SMD) Samsung S6D27A1 panel.
This display panel is used on Samsung mobile devices such as
Samsung Galaxy Ace 2 (GT-I8160) also known as Codina.

Markuss Broks (2):
  drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel

 .../display/panel/samsung,s6d27a1.yaml        |  97 ++++++
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 323 ++++++++++++++++++
 4 files changed, 432 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c

-- 
2.32.0


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

* [PATCH 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  2021-08-05 13:33       ` [PATCH 0/2] Add support " Markuss Broks
@ 2021-08-05 13:33         ` Markuss Broks
  2021-08-05 19:18           ` Sam Ravnborg
  2021-08-05 13:33         ` [PATCH 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
  1 sibling, 1 reply; 21+ messages in thread
From: Markuss Broks @ 2021-08-05 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linus.walleij, phone-devel, Markuss Broks, Thierry Reding,
	Sam Ravnborg, David Airlie, Daniel Vetter, Rob Herring,
	dri-devel, devicetree

This adds device-tree bindings for the Samsung S6D27A1 RGB
DPI display panel.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../display/panel/samsung,s6d27a1.yaml        | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
new file mode 100644
index 0000000000..c6c534aa87
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,s6d27a1.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung S6D27A1 display panel
+
+description: The S6D27A1 is a 480x800 DPI display panel from Samsung Mobile
+  Displays (SMD). The panel must obey the rules for a SPI slave device
+  as specified in spi/spi-controller.yaml
+
+maintainers:
+  - Markuss Broks <markuss.broks@gmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,s6d27a1
+
+  reg: true
+
+  interrupts:
+    description: provides an optional ESD (electrostatic discharge)
+      interrupt that signals abnormalities in the display hardware.
+      This can also be raised for other reasons like erroneous
+      configuration.
+    maxItems: 1
+
+  reset-gpios: true
+
+  vci-supply:
+    description: regulator that supplies the VCI analog voltage
+      usually around 3.0 V
+
+  vccio-supply:
+    description: regulator that supplies the VCCIO voltage usually
+      around 1.8 V
+
+  backlight: true
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  spi-max-frequency:
+    maximum: 1200000
+
+  port: true
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+  - spi-cpol
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        compatible = "spi-gpio";
+        sck-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+        miso-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+        mosi-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+        cs-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
+        num-chipselects = <1>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        panel@0 {
+            compatible = "samsung,s6d27a1";
+            spi-max-frequency = <1200000>;
+            spi-cpha;
+            spi-cpol;
+            reg = <0>;
+            vci-supply = <&lcd_3v0_reg>;
+            vccio-supply = <&lcd_1v8_reg>;
+            reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
+            interrupt-parent = <&gpio>;
+            interrupts = <5 IRQ_TYPE_EDGE_RISING>;
+
+            port {
+                panel_in: endpoint {
+                    remote-endpoint = <&display_out>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.32.0


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

* [PATCH 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-05 13:33       ` [PATCH 0/2] Add support " Markuss Broks
  2021-08-05 13:33         ` [PATCH 1/2] drm/panel: Add DT bindings " Markuss Broks
@ 2021-08-05 13:33         ` Markuss Broks
  2021-08-05 19:25           ` Sam Ravnborg
  2021-08-05 21:06             ` Linus Walleij
  1 sibling, 2 replies; 21+ messages in thread
From: Markuss Broks @ 2021-08-05 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linus.walleij, phone-devel, Markuss Broks, Thierry Reding,
	Sam Ravnborg, David Airlie, Daniel Vetter, Rob Herring,
	dri-devel, devicetree

This adds a driver for Samsung S6D27A1 display controller and panel.
This panel is found in the Samsung GT-I8160 mobile phone,
and possibly some other mobile phones.

This display needs manufacturer commands to configure it;
the commands used in this driver were taken from downstream driver
by Gareth Phillips; sadly, there is almost no documentation on what they
actually do.

This driver re-uses the DBI infrastructure to communicate with the display.

This driver is heavily based on WideChips WS2401 display controller
driver by Linus Walleij and on other panel drivers for reference.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>

v2 -> v3:
fixed checkpatch warnings - not sure about MAINTAINERS, should I
put my name up there?
---
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 323 ++++++++++++++++++
 3 files changed, 335 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index beb581b96e..0b37849413 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -392,6 +392,17 @@ config DRM_PANEL_SAMSUNG_S6D16D0
 	depends on DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
+config DRM_PANEL_SAMSUNG_S6D27A1
+	tristate "Samsung S6D27A1 DPI panel driver"
+	depends on OF && SPI && GPIOLIB
+	select DRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for the Samsung
+	  S6D27A1 DPI 480x800 panel.
+
+	  This panel can be found in Samsung Galaxy Ace 2
+	  GT-I8160 mobile phone.
+
 config DRM_PANEL_SAMSUNG_S6E3HA2
 	tristate "Samsung S6E3HA2 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index c8132050bc..60c0149fc5 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -39,6 +39,7 @@ 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
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
new file mode 100644
index 0000000000..6a77a15136
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Panel driver for the Samsung S6D27A1 480x800 DPI RGB panel.
+ * Found in the Samsung Galaxy Ace 2 GT-I8160 mobile phone.
+ */
+
+#include <drm/drm_modes.h>
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_panel.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/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+#define S6D27A1_PASSWD_L2	0xF0	/* Password Command for Level 2 Control */
+#define S6D27A1_RESCTL		0xB3	/* Resolution Select Control */
+#define S6D27A1_PANELCTL2	0xB4	/* ASG Signal Control */
+#define S6D27A1_READID1		0xDA	/* Read panel ID 1 */
+#define S6D27A1_READID2		0xDB	/* Read panel ID 2 */
+#define S6D27A1_READID3		0xDC	/* Read panel ID 3 */
+#define S6D27A1_DISPCTL		0xF2	/* Display Control */
+#define S6D27A1_MANPWR		0xF3	/* Manual Control */
+#define S6D27A1_PWRCTL1		0xF4	/* Power Control */
+#define S6D27A1_SRCCTL		0xF6	/* Source Control */
+#define S6D27A1_PANELCTL	0xF7	/* Panel Control*/
+
+static const u8 s6d27a1_dbi_read_commands[] = {
+	S6D27A1_READID1,
+	S6D27A1_READID2,
+	S6D27A1_READID3,
+	0, /* sentinel */
+};
+
+struct s6d27a1 {
+	struct device *dev;
+	struct mipi_dbi dbi;
+	struct drm_panel panel;
+	struct gpio_desc *reset;
+	struct regulator_bulk_data regulators[2];
+};
+
+static const struct drm_display_mode s6d27a1_480_800_mode = {
+	/*
+	 * The vendor driver states that the S6D27A1 panel
+	 * has a pixel clock frequency of 49920000 Hz / 2 = 24960000 Hz.
+	 */
+	.clock = 24960,
+	.hdisplay = 480,
+	.hsync_start = 480 + 63,
+	.hsync_end = 480 + 63 + 2,
+	.htotal = 480 + 63 + 2 + 63,
+	.vdisplay = 800,
+	.vsync_start = 800 + 11,
+	.vsync_end = 800 + 11 + 2,
+	.vtotal = 800 + 11 + 2 + 10,
+	.width_mm = 50,
+	.height_mm = 84,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static inline struct s6d27a1 *to_s6d27a1(struct drm_panel *panel)
+{
+	return container_of(panel, struct s6d27a1, panel);
+}
+
+#define s6d27a1_command(ctx, cmd, seq...) \
+({ \
+	struct mipi_dbi *dbi = &ctx->dbi; \
+	int ret; \
+	ret = mipi_dbi_command(dbi, cmd, seq);	\
+	if (ret) { \
+		dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
+	} \
+})
+
+static void s6d27a1_read_mtp_id(struct s6d27a1 *ctx)
+{
+	struct mipi_dbi *dbi = &ctx->dbi;
+	u8 id1, id2, id3;
+	int ret;
+
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID1, &id1);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 1\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID2, &id2);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 2\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID3, &id3);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 3\n");
+		return;
+	}
+	dev_info(ctx->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
+}
+
+static int s6d27a1_power_on(struct s6d27a1 *ctx)
+{
+	int ret;
+
+	/* Power up */
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->regulators),
+				    ctx->regulators);
+	if (ret) {
+		dev_err(ctx->dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	msleep(20);
+
+	/* Assert reset >=1 ms */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	usleep_range(1000, 5000);
+	/* De-assert reset */
+	gpiod_set_value_cansleep(ctx->reset, 0);
+	/* Wait >= 10 ms */
+	msleep(20);
+
+	/*
+	 * Exit sleep mode and initialize display - some hammering is
+	 * necessary.
+	 */
+	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(50);
+
+	/* Magic to unlock level 2 control of the display */
+	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0x5A, 0x5A);
+
+	/* Configure resolution to 480RGBx800 */
+	s6d27a1_command(ctx, S6D27A1_RESCTL, 0x22);
+
+	s6d27a1_command(ctx, S6D27A1_PANELCTL2, 0x00, 0x02, 0x03, 0x04, 0x05, 0x08, 0x00, 0x0c);
+
+	s6d27a1_command(ctx, S6D27A1_MANPWR, 0x01, 0x00, 0x00, 0x08, 0x08, 0x02, 0x00);
+
+	s6d27a1_command(ctx, S6D27A1_DISPCTL, 0x19, 0x00, 0x08, 0x0D, 0x03, 0x41, 0x3F);
+
+	s6d27a1_command(ctx, S6D27A1_PWRCTL1, 0x00, 0x00, 0x00, 0x00, 0x55,
+					0x44, 0x05, 0x88, 0x4B, 0x50);
+
+	s6d27a1_command(ctx, S6D27A1_SRCCTL, 0x03, 0x09, 0x8A, 0x00, 0x01, 0x16);
+
+	s6d27a1_command(ctx, S6D27A1_PANELCTL, 0x00, 0x05, 0x06, 0x07, 0x08,
+					0x01, 0x09, 0x0D, 0x0A, 0x0E,
+					0x0B, 0x0F, 0x0C, 0x10, 0x01,
+					0x11, 0x12, 0x13, 0x14, 0x05,
+					0x06, 0x07, 0x08, 0x01, 0x09,
+					0x0D, 0x0A, 0x0E, 0x0B, 0x0F,
+					0x0C, 0x10, 0x01, 0x11, 0x12,
+					0x13, 0x14);
+
+	/* lock the level 2 control */
+	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0xA5, 0xA5);
+
+	s6d27a1_read_mtp_id(ctx);
+
+	return 0;
+}
+
+static int s6d27a1_power_off(struct s6d27a1 *ctx)
+{
+	/* Go into RESET and disable regulators */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+}
+
+static int s6d27a1_unprepare(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(120);
+	return s6d27a1_power_off(to_s6d27a1(panel));
+}
+
+static int s6d27a1_disable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(25);
+
+	return 0;
+}
+
+static int s6d27a1_prepare(struct drm_panel *panel)
+{
+	return s6d27a1_power_on(to_s6d27a1(panel));
+}
+
+static int s6d27a1_enable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+
+	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_ON);
+
+	return 0;
+}
+
+static int s6d27a1_get_modes(struct drm_panel *panel,
+			    struct drm_connector *connector)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, &s6d27a1_480_800_mode);
+	if (!mode) {
+		dev_err(ctx->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_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 s6d27a1_drm_funcs = {
+	.disable = s6d27a1_disable,
+	.unprepare = s6d27a1_unprepare,
+	.prepare = s6d27a1_prepare,
+	.enable = s6d27a1_enable,
+	.get_modes = s6d27a1_get_modes,
+};
+
+static int s6d27a1_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct s6d27a1 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->dev = dev;
+
+	/*
+	 * VCI   is the analog voltage supply
+	 * VCCIO is the digital I/O voltage supply
+	 */
+	ctx->regulators[0].supply = "vci";
+	ctx->regulators[1].supply = "vccio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	ctx->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset)) {
+		ret = PTR_ERR(ctx->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &ctx->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	ctx->dbi.read_commands = s6d27a1_dbi_read_commands;
+
+	drm_panel_init(&ctx->panel, dev, &s6d27a1_drm_funcs,
+		       DRM_MODE_CONNECTOR_DPI);
+
+	spi_set_drvdata(spi, ctx);
+
+	drm_panel_add(&ctx->panel);
+	dev_dbg(dev, "added panel\n");
+
+	return 0;
+}
+
+static int s6d27a1_remove(struct spi_device *spi)
+{
+	struct s6d27a1 *ctx = spi_get_drvdata(spi);
+
+	drm_panel_remove(&ctx->panel);
+	return 0;
+}
+
+static const struct of_device_id s6d27a1_match[] = {
+	{ .compatible = "samsung,s6d27a1", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s6d27a1_match);
+
+static struct spi_driver s6d27a1_driver = {
+	.probe		= s6d27a1_probe,
+	.remove		= s6d27a1_remove,
+	.driver		= {
+		.name	= "s6d27a1-panel",
+		.of_match_table = s6d27a1_match,
+	},
+};
+module_spi_driver(s6d27a1_driver);
+
+MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>");
+MODULE_DESCRIPTION("Samsung S6D27A1 panel driver");
+MODULE_LICENSE("GPL v2");
-- 
2.32.0


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

* Re: [PATCH 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  2021-08-05 13:33         ` [PATCH 1/2] drm/panel: Add DT bindings " Markuss Broks
@ 2021-08-05 19:18           ` Sam Ravnborg
  0 siblings, 0 replies; 21+ messages in thread
From: Sam Ravnborg @ 2021-08-05 19:18 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, linus.walleij, phone-devel, Thierry Reding,
	David Airlie, Daniel Vetter, Rob Herring, dri-devel, devicetree

Hi Markuss,

On Thu, Aug 05, 2021 at 04:33:41PM +0300, Markuss Broks wrote:
> This adds device-tree bindings for the Samsung S6D27A1 RGB
> DPI display panel.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../display/panel/samsung,s6d27a1.yaml        | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> new file mode 100644
> index 0000000000..c6c534aa87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,s6d27a1.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung S6D27A1 display panel
> +
> +description: The S6D27A1 is a 480x800 DPI display panel from Samsung Mobile
> +  Displays (SMD). The panel must obey the rules for a SPI slave device
> +  as specified in spi/spi-controller.yaml
> +
> +maintainers:
> +  - Markuss Broks <markuss.broks@gmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: samsung,s6d27a1
> +
> +  reg: true
> +
> +  interrupts:
> +    description: provides an optional ESD (electrostatic discharge)
> +      interrupt that signals abnormalities in the display hardware.
> +      This can also be raised for other reasons like erroneous
> +      configuration.
> +    maxItems: 1
> +
> +  reset-gpios: true
> +
> +  vci-supply:
> +    description: regulator that supplies the VCI analog voltage
> +      usually around 3.0 V
> +
> +  vccio-supply:
> +    description: regulator that supplies the VCCIO voltage usually
> +      around 1.8 V
> +
> +  backlight: true
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  spi-max-frequency:
> +    maximum: 1200000
> +
> +  port: true
> +
> +required:
> +  - compatible
> +  - reg

> +  - spi-cpha
> +  - spi-cpol
> +  - port
Do the HW really require the above three properties?

Can the HW work without a supply (hint: I think vci-supply and
vccio-supply are required)

> +
> +additionalProperties: false

As this device sits on the SPI bus you need to use:
unevaluatedProterties: false
(Check spelling from other bindings)

This allows DT files to add more spi related properties, but do not
allow DT files to add unknown properties.

With the current scheme you restrict the DT file to only the three
mentioned SPI properties that may be OK for your HW but maybe not OK for
the next HW using the same panel.

	Sam



> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        compatible = "spi-gpio";
> +        sck-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
> +        miso-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
> +        mosi-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
> +        cs-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
> +        num-chipselects = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        panel@0 {
> +            compatible = "samsung,s6d27a1";
> +            spi-max-frequency = <1200000>;
> +            spi-cpha;
> +            spi-cpol;
> +            reg = <0>;
> +            vci-supply = <&lcd_3v0_reg>;
> +            vccio-supply = <&lcd_1v8_reg>;
> +            reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <5 IRQ_TYPE_EDGE_RISING>;
> +
> +            port {
> +                panel_in: endpoint {
> +                    remote-endpoint = <&display_out>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.32.0

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

* Re: [PATCH 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-05 13:33         ` [PATCH 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
@ 2021-08-05 19:25           ` Sam Ravnborg
  2021-08-07 13:31             ` [PATCH 0/2] Add support " Markuss Broks
  2021-08-05 21:06             ` Linus Walleij
  1 sibling, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2021-08-05 19:25 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, linus.walleij, phone-devel, Thierry Reding,
	David Airlie, Daniel Vetter, Rob Herring, dri-devel, devicetree

Hi Markuss,

On Thu, Aug 05, 2021 at 04:33:42PM +0300, Markuss Broks wrote:
> This adds a driver for Samsung S6D27A1 display controller and panel.
> This panel is found in the Samsung GT-I8160 mobile phone,
> and possibly some other mobile phones.
> 
> This display needs manufacturer commands to configure it;
> the commands used in this driver were taken from downstream driver
> by Gareth Phillips; sadly, there is almost no documentation on what they
> actually do.
> 
> This driver re-uses the DBI infrastructure to communicate with the display.
> 
> This driver is heavily based on WideChips WS2401 display controller
> driver by Linus Walleij and on other panel drivers for reference.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> 
> v2 -> v3:
> fixed checkpatch warnings - not sure about MAINTAINERS, should I
> put my name up there?
You made lots of other changes..

Yes, MAINTAINERS would be great so you get mails when something is
updated. This allows you to verify refactoring changes or bug fixes for
example.

For a single driver you can realy on someone else to apply patches, no
need to deal with drm-misc etc. for that.

A few details in the following, but driver is almost there.

	Sam


> ---
>  drivers/gpu/drm/panel/Kconfig                 |  11 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 323 ++++++++++++++++++
>  3 files changed, 335 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index beb581b96e..0b37849413 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -392,6 +392,17 @@ config DRM_PANEL_SAMSUNG_S6D16D0
>  	depends on DRM_MIPI_DSI
>  	select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_SAMSUNG_S6D27A1
> +	tristate "Samsung S6D27A1 DPI panel driver"
> +	depends on OF && SPI && GPIOLIB
> +	select DRM_MIPI_DBI
> +	help
> +	  Say Y here if you want to enable support for the Samsung
> +	  S6D27A1 DPI 480x800 panel.
> +
> +	  This panel can be found in Samsung Galaxy Ace 2
> +	  GT-I8160 mobile phone.
> +
>  config DRM_PANEL_SAMSUNG_S6E3HA2
>  	tristate "Samsung S6E3HA2 DSI video mode panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index c8132050bc..60c0149fc5 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -39,6 +39,7 @@ 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
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
> new file mode 100644
> index 0000000000..6a77a15136
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
> @@ -0,0 +1,323 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Panel driver for the Samsung S6D27A1 480x800 DPI RGB panel.
> + * Found in the Samsung Galaxy Ace 2 GT-I8160 mobile phone.
> + */
> +
> +#include <drm/drm_modes.h>
> +#include <drm/drm_mipi_dbi.h>
> +#include <drm/drm_panel.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/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <video/mipi_display.h>
> +
> +#define S6D27A1_PASSWD_L2	0xF0	/* Password Command for Level 2 Control */
> +#define S6D27A1_RESCTL		0xB3	/* Resolution Select Control */
> +#define S6D27A1_PANELCTL2	0xB4	/* ASG Signal Control */
> +#define S6D27A1_READID1		0xDA	/* Read panel ID 1 */
> +#define S6D27A1_READID2		0xDB	/* Read panel ID 2 */
> +#define S6D27A1_READID3		0xDC	/* Read panel ID 3 */
> +#define S6D27A1_DISPCTL		0xF2	/* Display Control */
> +#define S6D27A1_MANPWR		0xF3	/* Manual Control */
> +#define S6D27A1_PWRCTL1		0xF4	/* Power Control */
> +#define S6D27A1_SRCCTL		0xF6	/* Source Control */
> +#define S6D27A1_PANELCTL	0xF7	/* Panel Control*/
> +
> +static const u8 s6d27a1_dbi_read_commands[] = {
> +	S6D27A1_READID1,
> +	S6D27A1_READID2,
> +	S6D27A1_READID3,
> +	0, /* sentinel */
> +};
> +
> +struct s6d27a1 {
> +	struct device *dev;
> +	struct mipi_dbi dbi;
> +	struct drm_panel panel;
> +	struct gpio_desc *reset;
> +	struct regulator_bulk_data regulators[2];
> +};
> +
> +static const struct drm_display_mode s6d27a1_480_800_mode = {
> +	/*
> +	 * The vendor driver states that the S6D27A1 panel
> +	 * has a pixel clock frequency of 49920000 Hz / 2 = 24960000 Hz.
> +	 */
> +	.clock = 24960,
> +	.hdisplay = 480,
> +	.hsync_start = 480 + 63,
> +	.hsync_end = 480 + 63 + 2,
> +	.htotal = 480 + 63 + 2 + 63,
> +	.vdisplay = 800,
> +	.vsync_start = 800 + 11,
> +	.vsync_end = 800 + 11 + 2,
> +	.vtotal = 800 + 11 + 2 + 10,
> +	.width_mm = 50,
> +	.height_mm = 84,
> +	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +};
> +
> +static inline struct s6d27a1 *to_s6d27a1(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct s6d27a1, panel);
> +}
> +
> +#define s6d27a1_command(ctx, cmd, seq...) \
> +({ \
> +	struct mipi_dbi *dbi = &ctx->dbi; \
> +	int ret; \
> +	ret = mipi_dbi_command(dbi, cmd, seq);	\
> +	if (ret) { \
> +		dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
> +	} \
> +})
This does not error out like other panel drivers. I think this is
wrong.

> +
> +static void s6d27a1_read_mtp_id(struct s6d27a1 *ctx)
> +{
> +	struct mipi_dbi *dbi = &ctx->dbi;
> +	u8 id1, id2, id3;
> +	int ret;
> +
> +	ret = mipi_dbi_command_read(dbi, S6D27A1_READID1, &id1);
> +	if (ret) {
> +		dev_err(ctx->dev, "unable to read MTP ID 1\n");
> +		return;
> +	}
> +	ret = mipi_dbi_command_read(dbi, S6D27A1_READID2, &id2);
> +	if (ret) {
> +		dev_err(ctx->dev, "unable to read MTP ID 2\n");
> +		return;
> +	}
> +	ret = mipi_dbi_command_read(dbi, S6D27A1_READID3, &id3);
> +	if (ret) {
> +		dev_err(ctx->dev, "unable to read MTP ID 3\n");
> +		return;
> +	}
> +	dev_info(ctx->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
> +}
> +
> +static int s6d27a1_power_on(struct s6d27a1 *ctx)
> +{
> +	int ret;
> +
> +	/* Power up */
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->regulators),
> +				    ctx->regulators);
> +	if (ret) {
> +		dev_err(ctx->dev, "failed to enable regulators: %d\n", ret);
> +		return ret;
> +	}
> +
> +	msleep(20);
> +
> +	/* Assert reset >=1 ms */
> +	gpiod_set_value_cansleep(ctx->reset, 1);
> +	usleep_range(1000, 5000);
> +	/* De-assert reset */
> +	gpiod_set_value_cansleep(ctx->reset, 0);
> +	/* Wait >= 10 ms */
> +	msleep(20);
> +
> +	/*
> +	 * Exit sleep mode and initialize display - some hammering is
> +	 * necessary.
> +	 */
> +	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
> +	s6d27a1_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
> +	msleep(50);
> +
> +	/* Magic to unlock level 2 control of the display */
> +	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0x5A, 0x5A);
> +
> +	/* Configure resolution to 480RGBx800 */
> +	s6d27a1_command(ctx, S6D27A1_RESCTL, 0x22);
> +
> +	s6d27a1_command(ctx, S6D27A1_PANELCTL2, 0x00, 0x02, 0x03, 0x04, 0x05, 0x08, 0x00, 0x0c);
> +
> +	s6d27a1_command(ctx, S6D27A1_MANPWR, 0x01, 0x00, 0x00, 0x08, 0x08, 0x02, 0x00);
> +
> +	s6d27a1_command(ctx, S6D27A1_DISPCTL, 0x19, 0x00, 0x08, 0x0D, 0x03, 0x41, 0x3F);
> +
> +	s6d27a1_command(ctx, S6D27A1_PWRCTL1, 0x00, 0x00, 0x00, 0x00, 0x55,
> +					0x44, 0x05, 0x88, 0x4B, 0x50);
> +
> +	s6d27a1_command(ctx, S6D27A1_SRCCTL, 0x03, 0x09, 0x8A, 0x00, 0x01, 0x16);
> +
> +	s6d27a1_command(ctx, S6D27A1_PANELCTL, 0x00, 0x05, 0x06, 0x07, 0x08,
> +					0x01, 0x09, 0x0D, 0x0A, 0x0E,
> +					0x0B, 0x0F, 0x0C, 0x10, 0x01,
> +					0x11, 0x12, 0x13, 0x14, 0x05,
> +					0x06, 0x07, 0x08, 0x01, 0x09,
> +					0x0D, 0x0A, 0x0E, 0x0B, 0x0F,
> +					0x0C, 0x10, 0x01, 0x11, 0x12,
> +					0x13, 0x14);
> +
> +	/* lock the level 2 control */
> +	s6d27a1_command(ctx, S6D27A1_PASSWD_L2, 0xA5, 0xA5);
> +
> +	s6d27a1_read_mtp_id(ctx);
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_power_off(struct s6d27a1 *ctx)
> +{
> +	/* Go into RESET and disable regulators */
> +	gpiod_set_value_cansleep(ctx->reset, 1);
> +	return regulator_bulk_disable(ARRAY_SIZE(ctx->regulators),
> +				      ctx->regulators);
> +}
> +
> +static int s6d27a1_unprepare(struct drm_panel *panel)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +
> +	s6d27a1_command(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
> +	msleep(120);
> +	return s6d27a1_power_off(to_s6d27a1(panel));
> +}
> +
> +static int s6d27a1_disable(struct drm_panel *panel)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +
> +	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_OFF);
> +	msleep(25);
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_prepare(struct drm_panel *panel)
> +{
> +	return s6d27a1_power_on(to_s6d27a1(panel));
> +}
> +
> +static int s6d27a1_enable(struct drm_panel *panel)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +
> +	s6d27a1_command(ctx, MIPI_DCS_SET_DISPLAY_ON);
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_get_modes(struct drm_panel *panel,
> +			    struct drm_connector *connector)
> +{
> +	struct s6d27a1 *ctx = to_s6d27a1(panel);
> +	struct drm_display_mode *mode;
> +	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +
> +	mode = drm_mode_duplicate(connector->dev, &s6d27a1_480_800_mode);
> +	if (!mode) {
> +		dev_err(ctx->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_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 s6d27a1_drm_funcs = {
> +	.disable = s6d27a1_disable,
> +	.unprepare = s6d27a1_unprepare,
> +	.prepare = s6d27a1_prepare,
> +	.enable = s6d27a1_enable,
> +	.get_modes = s6d27a1_get_modes,
> +};
> +
> +static int s6d27a1_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct s6d27a1 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->dev = dev;
> +
> +	/*
> +	 * VCI   is the analog voltage supply
> +	 * VCCIO is the digital I/O voltage supply
> +	 */
> +	ctx->regulators[0].supply = "vci";
> +	ctx->regulators[1].supply = "vccio";
> +	ret = devm_regulator_bulk_get(dev,
> +				      ARRAY_SIZE(ctx->regulators),
> +				      ctx->regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	ctx->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->reset)) {
> +		ret = PTR_ERR(ctx->reset);
> +		return dev_err_probe(dev, ret, "no RESET GPIO\n");
> +	}
> +
> +	ret = mipi_dbi_spi_init(spi, &ctx->dbi, NULL);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> +
> +	ctx->dbi.read_commands = s6d27a1_dbi_read_commands;
> +
> +	drm_panel_init(&ctx->panel, dev, &s6d27a1_drm_funcs,
> +		       DRM_MODE_CONNECTOR_DPI);
> +
> +	spi_set_drvdata(spi, ctx);
> +
> +	drm_panel_add(&ctx->panel);
> +	dev_dbg(dev, "added panel\n");
Debug leftover that can be dropped.
> +
> +	return 0;
> +}
> +
> +static int s6d27a1_remove(struct spi_device *spi)
> +{
> +	struct s6d27a1 *ctx = spi_get_drvdata(spi);
> +
> +	drm_panel_remove(&ctx->panel);
> +	return 0;
> +}
> +
> +static const struct of_device_id s6d27a1_match[] = {
> +	{ .compatible = "samsung,s6d27a1", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, s6d27a1_match);
> +
> +static struct spi_driver s6d27a1_driver = {
> +	.probe		= s6d27a1_probe,
> +	.remove		= s6d27a1_remove,
> +	.driver		= {
> +		.name	= "s6d27a1-panel",
> +		.of_match_table = s6d27a1_match,
> +	},
> +};
> +module_spi_driver(s6d27a1_driver);
> +
> +MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>");
> +MODULE_DESCRIPTION("Samsung S6D27A1 panel driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.32.0

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

* Re: [PATCH 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-05 13:33         ` [PATCH 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
@ 2021-08-05 21:06             ` Linus Walleij
  2021-08-05 21:06             ` Linus Walleij
  1 sibling, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-05 21:06 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, phone-devel, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Rob Herring,
	open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Markuss,

sorry for reacting so late!

On Thu, Aug 5, 2021 at 3:36 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> +#define s6d27a1_command(ctx, cmd, seq...) \
> +({ \
> +       struct mipi_dbi *dbi = &ctx->dbi; \
> +       int ret; \
> +       ret = mipi_dbi_command(dbi, cmd, seq);  \
> +       if (ret) { \
> +               dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
> +       } \
> +})

You don't need this wrapper anymore, just call mipi_dbi_command() directly
everywhere you use s6d27a1_command().

Because I merged this patch:
https://cgit.freedesktop.org/drm/drm-misc/commit/include/drm/drm_mipi_dbi.h?id=3f5aa5ac0b0f9704f0c60f5fbbbcdc8c043d6eb6

Yours,
Linus Walleij

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

* Re: [PATCH 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
@ 2021-08-05 21:06             ` Linus Walleij
  0 siblings, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-05 21:06 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, phone-devel, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Rob Herring,
	open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Markuss,

sorry for reacting so late!

On Thu, Aug 5, 2021 at 3:36 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> +#define s6d27a1_command(ctx, cmd, seq...) \
> +({ \
> +       struct mipi_dbi *dbi = &ctx->dbi; \
> +       int ret; \
> +       ret = mipi_dbi_command(dbi, cmd, seq);  \
> +       if (ret) { \
> +               dev_err(ctx->dev, "failure in writing command %02x\n", cmd); \
> +       } \
> +})

You don't need this wrapper anymore, just call mipi_dbi_command() directly
everywhere you use s6d27a1_command().

Because I merged this patch:
https://cgit.freedesktop.org/drm/drm-misc/commit/include/drm/drm_mipi_dbi.h?id=3f5aa5ac0b0f9704f0c60f5fbbbcdc8c043d6eb6

Yours,
Linus Walleij

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

* [PATCH 0/2] Add support for Samsung S6D27A1 display panel
  2021-08-05 19:25           ` Sam Ravnborg
@ 2021-08-07 13:31             ` Markuss Broks
  2021-08-07 13:31               ` [PATCH v2 1/2] drm/panel: Add DT bindings " Markuss Broks
  2021-08-07 13:31               ` [PATCH v2 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
  0 siblings, 2 replies; 21+ messages in thread
From: Markuss Broks @ 2021-08-07 13:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: thierry.reding, airlied, daniel, robh+dt, dri-devel, devicetree,
	linus.walleij, phone-devel, Markuss Broks

Add support for Samsung Mobile Displays(SMD) Samsung S6D27A1 panel.
This display panel is used on Samsung mobile devices such as
Samsung Galaxy Ace 2 (GT-I8160) also known as Codina.

Markuss Broks (2):
  drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel

 .../display/panel/samsung,s6d27a1.yaml        |  97 ++++++
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 323 ++++++++++++++++++
 4 files changed, 432 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c

-- 
2.32.0


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

* [PATCH v2 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  2021-08-07 13:31             ` [PATCH 0/2] Add support " Markuss Broks
@ 2021-08-07 13:31               ` Markuss Broks
  2021-08-07 22:41                   ` Linus Walleij
  2021-08-13 20:41                 ` Rob Herring
  2021-08-07 13:31               ` [PATCH v2 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
  1 sibling, 2 replies; 21+ messages in thread
From: Markuss Broks @ 2021-08-07 13:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: thierry.reding, airlied, daniel, robh+dt, dri-devel, devicetree,
	linus.walleij, phone-devel, Markuss Broks

This adds device-tree bindings for the Samsung S6D27A1 RGB
DPI display panel.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>

v1 -> v2:
changed additionalProperties to unevaluatedProperties;
added vci-supply and vccio-supply as required;
---
 .../display/panel/samsung,s6d27a1.yaml        | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
new file mode 100644
index 0000000000..26e3c820a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/samsung,s6d27a1.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung S6D27A1 display panel
+
+description: The S6D27A1 is a 480x800 DPI display panel from Samsung Mobile
+  Displays (SMD). The panel must obey the rules for a SPI slave device
+  as specified in spi/spi-controller.yaml
+
+maintainers:
+  - Markuss Broks <markuss.broks@gmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: samsung,s6d27a1
+
+  reg: true
+
+  interrupts:
+    description: provides an optional ESD (electrostatic discharge)
+      interrupt that signals abnormalities in the display hardware.
+      This can also be raised for other reasons like erroneous
+      configuration.
+    maxItems: 1
+
+  reset-gpios: true
+
+  vci-supply:
+    description: regulator that supplies the VCI analog voltage
+      usually around 3.0 V
+
+  vccio-supply:
+    description: regulator that supplies the VCCIO voltage usually
+      around 1.8 V
+
+  backlight: true
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  spi-max-frequency:
+    maximum: 1200000
+
+  port: true
+
+required:
+  - compatible
+  - reg
+  - vci-supply
+  - vccio-supply
+  - spi-cpha
+  - spi-cpol
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        compatible = "spi-gpio";
+        sck-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+        miso-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+        mosi-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+        cs-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
+        num-chipselects = <1>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "samsung,s6d27a1";
+            spi-max-frequency = <1200000>;
+            spi-cpha;
+            spi-cpol;
+            reg = <0>;
+            vci-supply = <&lcd_3v0_reg>;
+            vccio-supply = <&lcd_1v8_reg>;
+            reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
+            interrupt-parent = <&gpio>;
+            interrupts = <5 IRQ_TYPE_EDGE_RISING>;
+
+            port {
+                panel_in: endpoint {
+                    remote-endpoint = <&display_out>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.32.0


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

* [PATCH v2 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-07 13:31             ` [PATCH 0/2] Add support " Markuss Broks
  2021-08-07 13:31               ` [PATCH v2 1/2] drm/panel: Add DT bindings " Markuss Broks
@ 2021-08-07 13:31               ` Markuss Broks
  2021-08-07 22:37                   ` Linus Walleij
  2021-08-13 13:54                   ` Linus Walleij
  1 sibling, 2 replies; 21+ messages in thread
From: Markuss Broks @ 2021-08-07 13:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: thierry.reding, airlied, daniel, robh+dt, dri-devel, devicetree,
	linus.walleij, phone-devel, Markuss Broks

This adds a driver for Samsung S6D27A1 display controller and panel.
This panel is found in the Samsung GT-I8160 mobile phone,
and possibly some other mobile phones.

This display needs manufacturer commands to configure it;
the commands used in this driver were taken from downstream driver
by Gareth Phillips; sadly, there is almost no documentation on what they
actually do.

This driver re-uses the DBI infrastructure to communicate with the display.

This driver is heavily based on WideChips WS2401 display controller
driver by Linus Walleij and on other panel drivers for reference.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>

v2 -> v3:
fixed checkpatch warnings - not sure about MAINTAINERS, should I
put my name up there?

v3 -> v4:

- add a MAINTAINERS entry;
- drop s6d27a1_command macro; use mipi_dbi_command directly;
- dropped "panel initialized" dev_dbg message;
- add drm_panel_of_backlight call to handle backlight
on panel turn on/off;
- reorder drm_mipi_dbi and drm_modes headers:
I think they should be in alphabetic order?
---
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c | 320 ++++++++++++++++++
 4 files changed, 338 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d27a1.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 851255b71c..c363209130 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5962,6 +5962,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml
 F:	drivers/gpu/drm/panel/panel-samsung-db7430.c
 
+DRM DRIVER FOR SAMSUNG S6D27A1 PANELS
+M:	Markuss Broks <markuss.broks@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
+F:	driver/gpu/drm/panel/panel-samsung-s6d27a1.c
+
 DRM DRIVER FOR SITRONIX ST7703 PANELS
 M:	Guido Günther <agx@sigxcpu.org>
 R:	Purism Kernel Team <kernel@puri.sm>
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index beb581b96e..0b37849413 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -392,6 +392,17 @@ config DRM_PANEL_SAMSUNG_S6D16D0
 	depends on DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
+config DRM_PANEL_SAMSUNG_S6D27A1
+	tristate "Samsung S6D27A1 DPI panel driver"
+	depends on OF && SPI && GPIOLIB
+	select DRM_MIPI_DBI
+	help
+	  Say Y here if you want to enable support for the Samsung
+	  S6D27A1 DPI 480x800 panel.
+
+	  This panel can be found in Samsung Galaxy Ace 2
+	  GT-I8160 mobile phone.
+
 config DRM_PANEL_SAMSUNG_S6E3HA2
 	tristate "Samsung S6E3HA2 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index c8132050bc..60c0149fc5 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -39,6 +39,7 @@ 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
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
new file mode 100644
index 0000000000..2dfcf48f2b
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d27a1.c
@@ -0,0 +1,320 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Panel driver for the Samsung S6D27A1 480x800 DPI RGB panel.
+ * Found in the Samsung Galaxy Ace 2 GT-I8160 mobile phone.
+ */
+
+#include <drm/drm_mipi_dbi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.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/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <video/mipi_display.h>
+
+#define S6D27A1_PASSWD_L2	0xF0	/* Password Command for Level 2 Control */
+#define S6D27A1_RESCTL		0xB3	/* Resolution Select Control */
+#define S6D27A1_PANELCTL2	0xB4	/* ASG Signal Control */
+#define S6D27A1_READID1		0xDA	/* Read panel ID 1 */
+#define S6D27A1_READID2		0xDB	/* Read panel ID 2 */
+#define S6D27A1_READID3		0xDC	/* Read panel ID 3 */
+#define S6D27A1_DISPCTL		0xF2	/* Display Control */
+#define S6D27A1_MANPWR		0xF3	/* Manual Control */
+#define S6D27A1_PWRCTL1		0xF4	/* Power Control */
+#define S6D27A1_SRCCTL		0xF6	/* Source Control */
+#define S6D27A1_PANELCTL	0xF7	/* Panel Control*/
+
+static const u8 s6d27a1_dbi_read_commands[] = {
+	S6D27A1_READID1,
+	S6D27A1_READID2,
+	S6D27A1_READID3,
+	0, /* sentinel */
+};
+
+struct s6d27a1 {
+	struct device *dev;
+	struct mipi_dbi dbi;
+	struct drm_panel panel;
+	struct gpio_desc *reset;
+	struct regulator_bulk_data regulators[2];
+};
+
+static const struct drm_display_mode s6d27a1_480_800_mode = {
+	/*
+	 * The vendor driver states that the S6D27A1 panel
+	 * has a pixel clock frequency of 49920000 Hz / 2 = 24960000 Hz.
+	 */
+	.clock = 24960,
+	.hdisplay = 480,
+	.hsync_start = 480 + 63,
+	.hsync_end = 480 + 63 + 2,
+	.htotal = 480 + 63 + 2 + 63,
+	.vdisplay = 800,
+	.vsync_start = 800 + 11,
+	.vsync_end = 800 + 11 + 2,
+	.vtotal = 800 + 11 + 2 + 10,
+	.width_mm = 50,
+	.height_mm = 84,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static inline struct s6d27a1 *to_s6d27a1(struct drm_panel *panel)
+{
+	return container_of(panel, struct s6d27a1, panel);
+}
+
+static void s6d27a1_read_mtp_id(struct s6d27a1 *ctx)
+{
+	struct mipi_dbi *dbi = &ctx->dbi;
+	u8 id1, id2, id3;
+	int ret;
+
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID1, &id1);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 1\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID2, &id2);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 2\n");
+		return;
+	}
+	ret = mipi_dbi_command_read(dbi, S6D27A1_READID3, &id3);
+	if (ret) {
+		dev_err(ctx->dev, "unable to read MTP ID 3\n");
+		return;
+	}
+	dev_info(ctx->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
+}
+
+static int s6d27a1_power_on(struct s6d27a1 *ctx)
+{
+	struct mipi_dbi *dbi = &ctx->dbi;
+	int ret;
+
+	/* Power up */
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->regulators),
+				    ctx->regulators);
+	if (ret) {
+		dev_err(ctx->dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	msleep(20);
+
+	/* Assert reset >=1 ms */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	usleep_range(1000, 5000);
+	/* De-assert reset */
+	gpiod_set_value_cansleep(ctx->reset, 0);
+	/* Wait >= 10 ms */
+	msleep(20);
+
+	/*
+	 * Exit sleep mode and initialize display - some hammering is
+	 * necessary.
+	 */
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(50);
+
+	/* Magic to unlock level 2 control of the display */
+	mipi_dbi_command(dbi, S6D27A1_PASSWD_L2, 0x5A, 0x5A);
+
+	/* Configure resolution to 480RGBx800 */
+	mipi_dbi_command(dbi, S6D27A1_RESCTL, 0x22);
+
+	mipi_dbi_command(dbi, S6D27A1_PANELCTL2, 0x00, 0x02, 0x03, 0x04, 0x05, 0x08, 0x00, 0x0c);
+
+	mipi_dbi_command(dbi, S6D27A1_MANPWR, 0x01, 0x00, 0x00, 0x08, 0x08, 0x02, 0x00);
+
+	mipi_dbi_command(dbi, S6D27A1_DISPCTL, 0x19, 0x00, 0x08, 0x0D, 0x03, 0x41, 0x3F);
+
+	mipi_dbi_command(dbi, S6D27A1_PWRCTL1, 0x00, 0x00, 0x00, 0x00, 0x55,
+					0x44, 0x05, 0x88, 0x4B, 0x50);
+
+	mipi_dbi_command(dbi, S6D27A1_SRCCTL, 0x03, 0x09, 0x8A, 0x00, 0x01, 0x16);
+
+	mipi_dbi_command(dbi, S6D27A1_PANELCTL, 0x00, 0x05, 0x06, 0x07, 0x08,
+					0x01, 0x09, 0x0D, 0x0A, 0x0E,
+					0x0B, 0x0F, 0x0C, 0x10, 0x01,
+					0x11, 0x12, 0x13, 0x14, 0x05,
+					0x06, 0x07, 0x08, 0x01, 0x09,
+					0x0D, 0x0A, 0x0E, 0x0B, 0x0F,
+					0x0C, 0x10, 0x01, 0x11, 0x12,
+					0x13, 0x14);
+
+	/* lock the level 2 control */
+	mipi_dbi_command(dbi, S6D27A1_PASSWD_L2, 0xA5, 0xA5);
+
+	s6d27a1_read_mtp_id(ctx);
+
+	return 0;
+}
+
+static int s6d27a1_power_off(struct s6d27a1 *ctx)
+{
+	/* Go into RESET and disable regulators */
+	gpiod_set_value_cansleep(ctx->reset, 1);
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+}
+
+static int s6d27a1_unprepare(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct mipi_dbi *dbi = &ctx->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(120);
+	return s6d27a1_power_off(to_s6d27a1(panel));
+}
+
+static int s6d27a1_disable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct mipi_dbi *dbi = &ctx->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(25);
+
+	return 0;
+}
+
+static int s6d27a1_prepare(struct drm_panel *panel)
+{
+	return s6d27a1_power_on(to_s6d27a1(panel));
+}
+
+static int s6d27a1_enable(struct drm_panel *panel)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct mipi_dbi *dbi = &ctx->dbi;
+
+	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
+
+	return 0;
+}
+
+static int s6d27a1_get_modes(struct drm_panel *panel,
+			    struct drm_connector *connector)
+{
+	struct s6d27a1 *ctx = to_s6d27a1(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, &s6d27a1_480_800_mode);
+	if (!mode) {
+		dev_err(ctx->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_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 s6d27a1_drm_funcs = {
+	.disable = s6d27a1_disable,
+	.unprepare = s6d27a1_unprepare,
+	.prepare = s6d27a1_prepare,
+	.enable = s6d27a1_enable,
+	.get_modes = s6d27a1_get_modes,
+};
+
+static int s6d27a1_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct s6d27a1 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->dev = dev;
+
+	/*
+	 * VCI   is the analog voltage supply
+	 * VCCIO is the digital I/O voltage supply
+	 */
+	ctx->regulators[0].supply = "vci";
+	ctx->regulators[1].supply = "vccio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(ctx->regulators),
+				      ctx->regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	ctx->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset)) {
+		ret = PTR_ERR(ctx->reset);
+		return dev_err_probe(dev, ret, "no RESET GPIO\n");
+	}
+
+	ret = mipi_dbi_spi_init(spi, &ctx->dbi, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
+
+	ctx->dbi.read_commands = s6d27a1_dbi_read_commands;
+
+	drm_panel_init(&ctx->panel, dev, &s6d27a1_drm_funcs,
+		       DRM_MODE_CONNECTOR_DPI);
+
+	ret = drm_panel_of_backlight(&ctx->panel);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add backlight\n");
+
+	spi_set_drvdata(spi, ctx);
+
+	drm_panel_add(&ctx->panel);
+
+	return 0;
+}
+
+static int s6d27a1_remove(struct spi_device *spi)
+{
+	struct s6d27a1 *ctx = spi_get_drvdata(spi);
+
+	drm_panel_remove(&ctx->panel);
+	return 0;
+}
+
+static const struct of_device_id s6d27a1_match[] = {
+	{ .compatible = "samsung,s6d27a1", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s6d27a1_match);
+
+static struct spi_driver s6d27a1_driver = {
+	.probe		= s6d27a1_probe,
+	.remove		= s6d27a1_remove,
+	.driver		= {
+		.name	= "s6d27a1-panel",
+		.of_match_table = s6d27a1_match,
+	},
+};
+module_spi_driver(s6d27a1_driver);
+
+MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>");
+MODULE_DESCRIPTION("Samsung S6D27A1 panel driver");
+MODULE_LICENSE("GPL v2");
-- 
2.32.0


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

* Re: [PATCH v2 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-07 13:31               ` [PATCH v2 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
@ 2021-08-07 22:37                   ` Linus Walleij
  2021-08-13 13:54                   ` Linus Walleij
  1 sibling, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-07 22:37 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, thierry.reding, Dave Airlie, Daniel Vetter,
	Rob Herring, open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	phone-devel

Hi Markuss,

this is looking really good, the following is just nitty gritty details
I could fix while applying, we're mostly waiting for DT review now.

On Sat, Aug 7, 2021 at 3:31 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> +       /*
> +        * Exit sleep mode and initialize display - some hammering is
> +        * necessary.
> +        */
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);

Check if this hammering is really necessary on s6d27a1, on Widechips
this is just a bug, what happens if you put just one of them?

> +       msleep(50);

This should be msleep(120) on s6d27a1 (according to board-codina-mcde.c)

Yours,
Linus Walleij

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

* Re: [PATCH v2 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
@ 2021-08-07 22:37                   ` Linus Walleij
  0 siblings, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-07 22:37 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, thierry.reding, Dave Airlie, Daniel Vetter,
	Rob Herring, open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	phone-devel

Hi Markuss,

this is looking really good, the following is just nitty gritty details
I could fix while applying, we're mostly waiting for DT review now.

On Sat, Aug 7, 2021 at 3:31 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> +       /*
> +        * Exit sleep mode and initialize display - some hammering is
> +        * necessary.
> +        */
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> +       mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);

Check if this hammering is really necessary on s6d27a1, on Widechips
this is just a bug, what happens if you put just one of them?

> +       msleep(50);

This should be msleep(120) on s6d27a1 (according to board-codina-mcde.c)

Yours,
Linus Walleij

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

* Re: [PATCH v2 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  2021-08-07 13:31               ` [PATCH v2 1/2] drm/panel: Add DT bindings " Markuss Broks
@ 2021-08-07 22:41                   ` Linus Walleij
  2021-08-13 20:41                 ` Rob Herring
  1 sibling, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-07 22:41 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, thierry.reding, Dave Airlie, Daniel Vetter,
	Rob Herring, open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	phone-devel

On Sat, Aug 7, 2021 at 3:31 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> This adds device-tree bindings for the Samsung S6D27A1 RGB
> DPI display panel.
>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>
> v1 -> v2:
> changed additionalProperties to unevaluatedProperties;
> added vci-supply and vccio-supply as required;

These bindings look good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Will give DT reviewers some slack before applying the patches.

Yours,
Linus Walleij

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

* Re: [PATCH v2 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel
@ 2021-08-07 22:41                   ` Linus Walleij
  0 siblings, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-07 22:41 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, thierry.reding, Dave Airlie, Daniel Vetter,
	Rob Herring, open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	phone-devel

On Sat, Aug 7, 2021 at 3:31 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> This adds device-tree bindings for the Samsung S6D27A1 RGB
> DPI display panel.
>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>
> v1 -> v2:
> changed additionalProperties to unevaluatedProperties;
> added vci-supply and vccio-supply as required;

These bindings look good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Will give DT reviewers some slack before applying the patches.

Yours,
Linus Walleij

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

* Re: [PATCH v2 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
  2021-08-07 13:31               ` [PATCH v2 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
@ 2021-08-13 13:54                   ` Linus Walleij
  2021-08-13 13:54                   ` Linus Walleij
  1 sibling, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-13 13:54 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, thierry.reding, Dave Airlie, Daniel Vetter,
	Rob Herring, open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	phone-devel

On Sat, Aug 7, 2021 at 3:31 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> This adds a driver for Samsung S6D27A1 display controller and panel.
> This panel is found in the Samsung GT-I8160 mobile phone,
> and possibly some other mobile phones.
>
> This display needs manufacturer commands to configure it;
> the commands used in this driver were taken from downstream driver
> by Gareth Phillips; sadly, there is almost no documentation on what they
> actually do.
>
> This driver re-uses the DBI infrastructure to communicate with the display.
>
> This driver is heavily based on WideChips WS2401 display controller
> driver by Linus Walleij and on other panel drivers for reference.
>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>
> v2 -> v3:

Both v3 patches applied to drm-misc-next and pushed.

Yours,
Linus Walleij

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

* Re: [PATCH v2 2/2] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel
@ 2021-08-13 13:54                   ` Linus Walleij
  0 siblings, 0 replies; 21+ messages in thread
From: Linus Walleij @ 2021-08-13 13:54 UTC (permalink / raw)
  To: Markuss Broks
  Cc: linux-kernel, thierry.reding, Dave Airlie, Daniel Vetter,
	Rob Herring, open list:DRM PANEL DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	phone-devel

On Sat, Aug 7, 2021 at 3:31 PM Markuss Broks <markuss.broks@gmail.com> wrote:

> This adds a driver for Samsung S6D27A1 display controller and panel.
> This panel is found in the Samsung GT-I8160 mobile phone,
> and possibly some other mobile phones.
>
> This display needs manufacturer commands to configure it;
> the commands used in this driver were taken from downstream driver
> by Gareth Phillips; sadly, there is almost no documentation on what they
> actually do.
>
> This driver re-uses the DBI infrastructure to communicate with the display.
>
> This driver is heavily based on WideChips WS2401 display controller
> driver by Linus Walleij and on other panel drivers for reference.
>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>
> v2 -> v3:

Both v3 patches applied to drm-misc-next and pushed.

Yours,
Linus Walleij

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

* Re: [PATCH v2 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel
  2021-08-07 13:31               ` [PATCH v2 1/2] drm/panel: Add DT bindings " Markuss Broks
  2021-08-07 22:41                   ` Linus Walleij
@ 2021-08-13 20:41                 ` Rob Herring
  1 sibling, 0 replies; 21+ messages in thread
From: Rob Herring @ 2021-08-13 20:41 UTC (permalink / raw)
  To: Markuss Broks
  Cc: thierry.reding, robh+dt, dri-devel, devicetree, daniel,
	phone-devel, linus.walleij, linux-kernel, airlied

On Sat, 07 Aug 2021 16:31:10 +0300, Markuss Broks wrote:
> This adds device-tree bindings for the Samsung S6D27A1 RGB
> DPI display panel.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> 
> v1 -> v2:
> changed additionalProperties to unevaluatedProperties;
> added vci-supply and vccio-supply as required;
> ---
>  .../display/panel/samsung,s6d27a1.yaml        | 98 +++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

end of thread, other threads:[~2021-08-13 20:41 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03 13:54 [PATCH] drm/panel: s6d27a1: Add driver for Samsung S6D27A1 display panel Markuss Broks
2021-08-03 14:45 ` Sam Ravnborg
2021-08-03 17:24   ` [PATCH v2] " Markuss Broks
2021-08-03 18:11     ` Sam Ravnborg
2021-08-05 13:33       ` [PATCH 0/2] Add support " Markuss Broks
2021-08-05 13:33         ` [PATCH 1/2] drm/panel: Add DT bindings " Markuss Broks
2021-08-05 19:18           ` Sam Ravnborg
2021-08-05 13:33         ` [PATCH 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
2021-08-05 19:25           ` Sam Ravnborg
2021-08-07 13:31             ` [PATCH 0/2] Add support " Markuss Broks
2021-08-07 13:31               ` [PATCH v2 1/2] drm/panel: Add DT bindings " Markuss Broks
2021-08-07 22:41                 ` Linus Walleij
2021-08-07 22:41                   ` Linus Walleij
2021-08-13 20:41                 ` Rob Herring
2021-08-07 13:31               ` [PATCH v2 2/2] drm/panel: s6d27a1: Add driver " Markuss Broks
2021-08-07 22:37                 ` Linus Walleij
2021-08-07 22:37                   ` Linus Walleij
2021-08-13 13:54                 ` Linus Walleij
2021-08-13 13:54                   ` Linus Walleij
2021-08-05 21:06           ` [PATCH " Linus Walleij
2021-08-05 21:06             ` Linus Walleij

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.