diff mbox series

[v3,2/3] drm/panel: Add Samsung S6D7AA0 panel controller driver

Message ID 20230416131632.31673-3-aweber.kernel@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add Samsung S6D7AA0 panel controller driver | expand

Commit Message

Artur Weber April 16, 2023, 1:16 p.m. UTC
Initial driver for S6D7AA0-controlled panels, currently only for the
LSL080AL02 panel used in the Samsung Galaxy Tab 3 8.0 family of tablets.

It should be possible to extend this driver to work with other panels
using this IC.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Changed in v2:
 - Removed unused panel_name property from desc struct
---
 drivers/gpu/drm/panel/Kconfig                 |   7 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c | 395 ++++++++++++++++++
 3 files changed, 403 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c

Comments

Linus Walleij April 20, 2023, 7:35 a.m. UTC | #1
Hi Artur,

thanks for your patch!

On Sun, Apr 16, 2023 at 3:16 PM Artur Weber <aweber.kernel@gmail.com> wrote:

> Initial driver for S6D7AA0-controlled panels, currently only for the
> LSL080AL02 panel used in the Samsung Galaxy Tab 3 8.0 family of tablets.
>
> It should be possible to extend this driver to work with other panels
> using this IC.
>
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
> Changed in v2:
>  - Removed unused panel_name property from desc struct

Overall this driver looks very good. I could merge it once the DT bindings
are ACKed by the DT maintainers and some minor stuff fixed.

Some comments below:

> +/* Manufacturer command set */
> +#define CMD_BL_CTL             0xc3
> +#define CMD_OTP_RELOAD         0xd0
> +#define CMD_PASSWD1            0xf0
> +#define CMD_PASSWD2            0xf1
> +#define CMD_PASSWD3            0xfc

Some drivers prefix these commands with "MCS" such as
MCS_BL_CTL.

MCS = Manufacturer Command Set (I think)

Some just name the identifers after the panel such as
s6d27a1 which has S6D27A1_RESCTL etc.

CMD seems a bit general to me and may be mistaken for
the actual DCS commands.

> +struct s6d7aa0 {
> +       struct drm_panel panel;
> +       struct mipi_dsi_device *dsi;
> +       struct gpio_desc *reset_gpio;
> +       struct regulator *enable_supply;
> +       const struct s6d7aa0_panel_desc *desc;
> +       bool prepared;

Skip this state variable, the core keeps track of whether the
panel is enabled or not.

> +static void s6d7aa0_reset(struct s6d7aa0 *ctx)
> +{
> +       gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +       msleep(50);

This first de-assertion is unnecessary isn't it?

The reset line will just be asserted longer if it is already asserted.

> +       gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +       msleep(50);
> +       gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +       msleep(50);
> +}

(...)

> +static int s6d7aa0_on(struct s6d7aa0 *ctx)
> +{
> +       struct mipi_dsi_device *dsi = ctx->dsi;
> +       struct device *dev = &dsi->dev;
> +       int ret;
> +
> +       dsi->mode_flags |= MIPI_DSI_MODE_LPM;

(...)

> +static int s6d7aa0_off(struct s6d7aa0 *ctx)
> +{
> +       struct mipi_dsi_device *dsi = ctx->dsi;
> +       struct device *dev = &dsi->dev;
> +       int ret;
> +
> +       dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;

I haven't seen this mode flag MIPI_DSI_MODE_LPM set and
masked in other DSI panel drivers! Is this something we should
fix everywhere then? Or even something the core should be
doing?

Yours,
Linus Walleij
Artur Weber April 22, 2023, 2:32 p.m. UTC | #2
Hi,

thank you for the review.

On 20/04/2023 09:35, Linus Walleij wrote:
>> +static int s6d7aa0_on(struct s6d7aa0 *ctx)
>> +{
>> +       struct mipi_dsi_device *dsi = ctx->dsi;
>> +       struct device *dev = &dsi->dev;
>> +       int ret;
>> +
>> +       dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> 
> (...)
> 
>> +static int s6d7aa0_off(struct s6d7aa0 *ctx)
>> +{
>> +       struct mipi_dsi_device *dsi = ctx->dsi;
>> +       struct device *dev = &dsi->dev;
>> +       int ret;
>> +
>> +       dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> 
> I haven't seen this mode flag MIPI_DSI_MODE_LPM set and
> masked in other DSI panel drivers! Is this something we should
> fix everywhere then? Or even something the core should be
> doing?

These bits were included in a driver for a similar panel with the same
controller in an MSM8916 close-to-mainline kernel fork[1]; that driver
was generated with lmdpdg[2], which adds the LPM mode flag automatically
based on some downstream DTS property. In this case, I left it in, since
it didn't seem to break anything... but I just re-tested without it and
it seems that it might've fixed some odd issues I'd get sometimes when
going out of sleep mode. I'll get rid of it in the next version.

(I based my panel driver off that driver; now that I think about it, it
might be worth mentioning somewhere in the copyright notice...?)

Best regards
Artur Weber

[1]
https://github.com/msm8916-mainline/linux/blob/msm8916/6.3-rc7/drivers/gpu/drm/panel/msm8916-generated/panel-samsung-s6d7aa0-lsl080al03.c
[2]
https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 8eeee71c0000..2ce9d1a45625 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -532,6 +532,13 @@  config DRM_PANEL_SAMSUNG_S6D27A1
 	  This panel can be found in Samsung Galaxy Ace 2
 	  GT-I8160 mobile phone.
 
+config DRM_PANEL_SAMSUNG_S6D7AA0
+	tristate "Samsung S6D7AA0 MIPI-DSI video mode panel controller"
+	depends on OF
+	depends on BACKLIGHT_CLASS_DEVICE
+	select DRM_MIPI_DSI
+	select VIDEOMODE_HELPERS
+
 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 c05aa9e23907..193f3067865d 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -52,6 +52,7 @@  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_S6D7AA0) += panel-samsung-s6d7aa0.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-s6d7aa0.c b/drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
new file mode 100644
index 000000000000..408e4082de74
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
@@ -0,0 +1,395 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Samsung S6D7AA0 MIPI-DSI TFT LCD controller drm_panel driver.
+ *
+ * Copyright (C) 2022 Artur Weber <aweber.kernel@gmail.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <video/mipi_display.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+/* Manufacturer command set */
+#define CMD_BL_CTL		0xc3
+#define CMD_OTP_RELOAD		0xd0
+#define CMD_PASSWD1		0xf0
+#define CMD_PASSWD2		0xf1
+#define CMD_PASSWD3		0xfc
+
+struct s6d7aa0 {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+	struct gpio_desc *reset_gpio;
+	struct regulator *enable_supply;
+	const struct s6d7aa0_panel_desc *desc;
+	bool prepared;
+};
+
+struct s6d7aa0_panel_desc {
+	int (*init_func)(struct s6d7aa0 *ctx);
+	const struct drm_display_mode drm_mode;
+	unsigned long mode_flags;
+	u32 bus_flags;
+	bool use_passwd3;
+};
+
+static inline struct s6d7aa0 *panel_to_s6d7aa0(struct drm_panel *panel)
+{
+	return container_of(panel, struct s6d7aa0, panel);
+}
+
+static void s6d7aa0_reset(struct s6d7aa0 *ctx)
+{
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	msleep(50);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	msleep(50);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	msleep(50);
+}
+
+static int s6d7aa0_lock(struct s6d7aa0 *ctx, bool lock)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+
+	if (lock) {
+		mipi_dsi_dcs_write_seq(dsi, CMD_PASSWD1, 0xa5, 0xa5);
+		mipi_dsi_dcs_write_seq(dsi, CMD_PASSWD2, 0xa5, 0xa5);
+		if (ctx->desc->use_passwd3)
+			mipi_dsi_dcs_write_seq(dsi, CMD_PASSWD3, 0x5a, 0x5a);
+	} else {
+		mipi_dsi_dcs_write_seq(dsi, CMD_PASSWD1, 0x5a, 0x5a);
+		mipi_dsi_dcs_write_seq(dsi, CMD_PASSWD2, 0x5a, 0x5a);
+		if (ctx->desc->use_passwd3)
+			mipi_dsi_dcs_write_seq(dsi, CMD_PASSWD3, 0xa5, 0xa5);
+	}
+
+	return 0;
+}
+
+static int s6d7aa0_bl_ctl_on(struct s6d7aa0 *ctx, bool enable)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+
+	if (enable)
+		mipi_dsi_dcs_write_seq(dsi, CMD_BL_CTL, 0x40, 0x00, 0x28);
+	else
+		mipi_dsi_dcs_write_seq(dsi, CMD_BL_CTL, 0x40, 0x00, 0x20);
+
+	return 0;
+}
+
+static int s6d7aa0_on(struct s6d7aa0 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	s6d7aa0_reset(ctx);
+
+	ret = ctx->desc->init_func(ctx);
+	if (ret < 0) {
+		dev_err(dev, "Failed to initialize panel: %d\n", ret);
+		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+		return ret;
+	}
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display on: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int s6d7aa0_off(struct s6d7aa0 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display off: %d\n", ret);
+		return ret;
+	}
+	msleep(64);
+
+	ret = s6d7aa0_bl_ctl_on(ctx, false);
+	if (ret < 0) {
+		dev_err(dev, "Failed to disable backlight control: %d\n", ret);
+		return ret;
+	}
+	usleep_range(1000, 1500);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+		return ret;
+	}
+	msleep(120);
+
+	return 0;
+}
+
+static int s6d7aa0_prepare(struct drm_panel *panel)
+{
+	struct s6d7aa0 *ctx = panel_to_s6d7aa0(panel);
+	struct device *dev = &ctx->dsi->dev;
+	int ret;
+
+	if (ctx->prepared)
+		return 0;
+
+	ret = regulator_enable(ctx->enable_supply);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulator: %d\n", ret);
+		return ret;
+	}
+
+	ret = s6d7aa0_on(ctx);
+	if (ret < 0) {
+		dev_err(dev, "Failed to initialize panel: %d\n", ret);
+		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+		return ret;
+	}
+
+	ctx->prepared = true;
+	return 0;
+}
+
+static int s6d7aa0_disable(struct drm_panel *panel)
+{
+	struct s6d7aa0 *ctx = panel_to_s6d7aa0(panel);
+	struct device *dev = &ctx->dsi->dev;
+	int ret;
+
+	ret = s6d7aa0_off(ctx);
+	if (ret < 0)
+		dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
+
+	return 0;
+}
+
+static int s6d7aa0_unprepare(struct drm_panel *panel)
+{
+	struct s6d7aa0 *ctx = panel_to_s6d7aa0(panel);
+
+	if (!ctx->prepared)
+		return 0;
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	regulator_disable(ctx->enable_supply);
+
+	ctx->prepared = false;
+	return 0;
+}
+
+/* Initialization code and structures for LSL080AL02 panel */
+
+static int s6d7aa0_lsl080al02_init(struct s6d7aa0 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	usleep_range(20000, 25000);
+
+	ret = s6d7aa0_lock(ctx, false);
+	if (ret < 0) {
+		dev_err(dev, "Failed to unlock registers: %d\n", ret);
+		return ret;
+	}
+
+	mipi_dsi_dcs_write_seq(dsi, CMD_OTP_RELOAD, 0x00, 0x10);
+	usleep_range(1000, 1500);
+
+	mipi_dsi_dcs_write_seq(dsi, 0xb6, 0x10); /* SEQ_B6_PARAM_8_R01 */
+
+	ret = s6d7aa0_bl_ctl_on(ctx, true);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable backlight control: %d\n", ret);
+		return ret;
+	}
+	usleep_range(5000, 6000);
+	mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_ADDRESS_MODE, 0x04);
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+		return ret;
+	}
+
+	msleep(120);
+	mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
+
+	ret = s6d7aa0_lock(ctx, true);
+	if (ret < 0) {
+		dev_err(dev, "Failed to lock registers: %d\n", ret);
+		return ret;
+	}
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display on: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};
+
+static const struct drm_display_mode s6d7aa0_lsl080al02_mode = {
+	.clock = (800 + 16 + 4 + 140) * (1280 + 8 + 4 + 4) * 60 / 1000,
+	.hdisplay = 800,
+	.hsync_start = 800 + 16,
+	.hsync_end = 800 + 16 + 4,
+	.htotal = 800 + 16 + 4 + 140,
+	.vdisplay = 1280,
+	.vsync_start = 1280 + 8,
+	.vsync_end = 1280 + 8 + 4,
+	.vtotal = 1280 + 8 + 4 + 4,
+	.width_mm = 108,
+	.height_mm = 173,
+};
+
+static const struct s6d7aa0_panel_desc s6d7aa0_lsl080al02_desc = {
+	.init_func = s6d7aa0_lsl080al02_init,
+	.drm_mode = s6d7aa0_lsl080al02_mode,
+	.mode_flags = MIPI_DSI_MODE_VSYNC_FLUSH,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
+	.use_passwd3 = false,
+};
+
+static int s6d7aa0_get_modes(struct drm_panel *panel,
+					struct drm_connector *connector)
+{
+	struct drm_display_mode *mode;
+	struct s6d7aa0 *ctx;
+
+	ctx = container_of(panel, struct s6d7aa0, panel);
+	if (!ctx)
+		return -EINVAL;
+
+	mode = drm_mode_duplicate(connector->dev, &ctx->desc->drm_mode);
+	if (!mode)
+		return -ENOMEM;
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	connector->display_info.bus_flags = ctx->desc->bus_flags;
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs s6d7aa0_panel_funcs = {
+	.disable = s6d7aa0_disable,
+	.prepare = s6d7aa0_prepare,
+	.unprepare = s6d7aa0_unprepare,
+	.get_modes = s6d7aa0_get_modes,
+};
+
+static int s6d7aa0_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct s6d7aa0 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->desc = of_device_get_match_data(dev);
+	if (!ctx->desc)
+		return -ENODEV;
+
+	ctx->enable_supply = devm_regulator_get(dev, "enable");
+	if (IS_ERR(ctx->enable_supply))
+		return dev_err_probe(dev, PTR_ERR(ctx->enable_supply),
+				     "Failed to get enable supply\n");
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+				     "Failed to get reset-gpios\n");
+
+	ctx->dsi = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
+		| ctx->desc->mode_flags;
+
+	drm_panel_init(&ctx->panel, dev, &s6d7aa0_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+	ctx->panel.prepare_prev_first = true;
+
+	ret = drm_panel_of_backlight(&ctx->panel);
+	if (ret) {
+		dev_err_probe(dev, ret, "Could not find backlight\n");
+		return ret;
+	}
+
+	drm_panel_add(&ctx->panel);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to attach to DSI host: %d\n", ret);
+		drm_panel_remove(&ctx->panel);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void s6d7aa0_remove(struct mipi_dsi_device *dsi)
+{
+	struct s6d7aa0 *ctx = mipi_dsi_get_drvdata(dsi);
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+	drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id s6d7aa0_of_match[] = {
+	{
+		.compatible = "samsung,s6d7aa0-lsl080al02",
+		.data = &s6d7aa0_lsl080al02_desc
+	},
+	{ /* sentinel */ }
+};
+
+static struct mipi_dsi_driver s6d7aa0_driver = {
+	.probe = s6d7aa0_probe,
+	.remove = s6d7aa0_remove,
+	.driver = {
+		.name = "panel-samsung-s6d7aa0",
+		.of_match_table = s6d7aa0_of_match,
+	},
+};
+module_mipi_dsi_driver(s6d7aa0_driver);
+
+MODULE_AUTHOR("Artur Weber <aweber.kernel@gmail.com>");
+MODULE_DESCRIPTION("Samsung S6D7AA0 MIPI-DSI LCD controller driver");
+MODULE_LICENSE("GPL");