From: Michael Srba Michael.Srba@seznam.cz
This patch adds dts bindings for Samsung AMS452EF01 AMOLED panel, which makes use of their S6E88A0 controller.
Signed-off-by: Michael Srba Michael.Srba@seznam.cz --- Hi, Thanks for the review. I believe I've fixed everything, and I tested that I get image on drm-next with these patches applied.
Changes since v1: use yaml format for the binding --- .../panel/samsung,s6e88a0-ams452ef01.yaml | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml
diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml new file mode 100644 index 000000000000..3d9b480ec706 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung AMS452EF01 AMOLED panel with S6E88A0 video mode DSI controller + +maintainers: + - Michael Srba Michael.Srba@seznam.cz + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + const: samsung,s6e88a0-ams452ef01 + reg: true + reset-gpios: true + vdd3-supply: + description: core voltage supply + vci-supply: + description: voltage supply for analog circuits + enforce-video-mode: true + +required: + - compatible + - reg + - vdd3-supply + - vci-supply + - reset-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + panel@0 { + reg = <0>; + + compatible = "samsung,s6e88a0-ams452ef01"; + + vdd3-supply = <&pm8916_l17>; + vci-supply = <®_vlcd_vci>; + reset-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>; + }; + +...
From: Michael Srba Michael.Srba@seznam.cz
Signed-off-by: Michael Srba Michael.Srba@seznam.cz --- Changes since v1: reorder includes; remove empty functions; fix after rebasing --- drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile | 1 + .../panel/panel-samsung-s6e88a0-ams452ef01.c | 289 ++++++++++++++++++ 3 files changed, 296 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index ae44ac2ec106..0c7d61f32b0e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -275,6 +275,12 @@ config DRM_PANEL_SAMSUNG_S6E63M0 Say Y here if you want to enable support for Samsung S6E63M0 AMOLED LCD panel.
+config DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01 + tristate "Samsung AMS452EF01 panel with S6E88A0 DSI video mode controller" + depends on OF + select DRM_MIPI_DSI + select VIDEOMODE_HELPERS + config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 7c4d3c581fd4..5b622fbe4014 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.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 +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01) += panel-samsung-s6e88a0-ams452ef01.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c new file mode 100644 index 000000000000..49d3f6d0f137 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c @@ -0,0 +1,289 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright (C) 2019, Michael Srba + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> + +#include <video/mipi_display.h> + +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> + +struct s6e88a0_ams452ef01 { + struct drm_panel panel; + struct mipi_dsi_device *dsi; + struct regulator_bulk_data supplies[2]; + struct gpio_desc *reset_gpio; + + bool prepared; +}; + +static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel) +{ + return container_of(panel, struct s6e88a0_ams452ef01, panel); +} + +#define dsi_dcs_write_seq(dsi, seq...) do { \ + static const u8 d[] = { seq }; \ + int ret; \ + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \ + if (ret < 0) \ + return ret; \ + } while (0) + +static void s6e88a0_ams452ef01_reset(struct s6e88a0_ams452ef01 *ctx) +{ + gpiod_set_value_cansleep(ctx->reset_gpio, 1); + usleep_range(5000, 6000); + gpiod_set_value_cansleep(ctx->reset_gpio, 0); + usleep_range(1000, 2000); + gpiod_set_value_cansleep(ctx->reset_gpio, 1); + usleep_range(10000, 11000); +} + +static int s6e88a0_ams452ef01_on(struct s6e88a0_ams452ef01 *ctx) +{ + struct mipi_dsi_device *dsi = ctx->dsi; + struct device *dev = &dsi->dev; + int ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + dsi_dcs_write_seq(dsi, 0xf0, 0x5a, 0x5a); // enable LEVEL2 commands + dsi_dcs_write_seq(dsi, 0xcc, 0x4c); // set Pixel Clock Divider polarity + + 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); + + // set default brightness/gama + dsi_dcs_write_seq(dsi, 0xca, + 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, // V255 RR,GG,BB + 0x80, 0x80, 0x80, // V203 R,G,B + 0x80, 0x80, 0x80, // V151 R,G,B + 0x80, 0x80, 0x80, // V87 R,G,B + 0x80, 0x80, 0x80, // V51 R,G,B + 0x80, 0x80, 0x80, // V35 R,G,B + 0x80, 0x80, 0x80, // V23 R,G,B + 0x80, 0x80, 0x80, // V11 R,G,B + 0x6b, 0x68, 0x71, // V3 R,G,B + 0x00, 0x00, 0x00); // V1 R,G,B + dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio + dsi_dcs_write_seq(dsi, 0xb6, 0x2c, 0x0b); // set default elvss voltage + dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, 0x00); + dsi_dcs_write_seq(dsi, 0xf7, 0x03); // gamma/aor update + dsi_dcs_write_seq(dsi, 0xf0, 0xa5, 0xa5); // disable LEVEL2 commands + + 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 s6e88a0_ams452ef01_off(struct s6e88a0_ams452ef01 *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(35); + + 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 s6e88a0_ams452ef01_prepare(struct drm_panel *panel) +{ + struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel); + struct device *dev = &ctx->dsi->dev; + int ret; + + if (ctx->prepared) + return 0; + + ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); + if (ret < 0) { + dev_err(dev, "Failed to enable regulators: %d\n", ret); + return ret; + } + + s6e88a0_ams452ef01_reset(ctx); + + ret = s6e88a0_ams452ef01_on(ctx); + if (ret < 0) { + dev_err(dev, "Failed to initialize panel: %d\n", ret); + gpiod_set_value_cansleep(ctx->reset_gpio, 0); + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); + return ret; + } + + ctx->prepared = true; + return 0; +} + +static int s6e88a0_ams452ef01_unprepare(struct drm_panel *panel) +{ + struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel); + struct device *dev = &ctx->dsi->dev; + int ret; + + if (!ctx->prepared) + return 0; + + ret = s6e88a0_ams452ef01_off(ctx); + if (ret < 0) + dev_err(dev, "Failed to un-initialize panel: %d\n", ret); + + gpiod_set_value_cansleep(ctx->reset_gpio, 0); + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); + + ctx->prepared = false; + return 0; +} + +static const struct drm_display_mode s6e88a0_ams452ef01_mode = { + .clock = (540 + 88 + 4 + 20) * (960 + 14 + 2 + 8) * 60 / 1000, + .hdisplay = 540, + .hsync_start = 540 + 88, + .hsync_end = 540 + 88 + 4, + .htotal = 540 + 88 + 4 + 20, + .vdisplay = 960, + .vsync_start = 960 + 14, + .vsync_end = 960 + 14 + 2, + .vtotal = 960 + 14 + 2 + 8, + .vrefresh = 60, + .width_mm = 56, + .height_mm = 100, +}; + +static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector) +{ + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(connector->dev, &s6e88a0_ams452ef01_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; + drm_mode_probed_add(connector, mode); + + return 1; +} + +static const struct drm_panel_funcs s6e88a0_ams452ef01_panel_funcs = { + .unprepare = s6e88a0_ams452ef01_unprepare, + .prepare = s6e88a0_ams452ef01_prepare, + .get_modes = s6e88a0_ams452ef01_get_modes, +}; + +static int s6e88a0_ams452ef01_probe(struct mipi_dsi_device *dsi) +{ + struct device *dev = &dsi->dev; + struct s6e88a0_ams452ef01 *ctx; + int ret; + + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->supplies[0].supply = "vdd3"; + ctx->supplies[1].supply = "vci"; + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies), + ctx->supplies); + if (ret < 0) { + dev_err(dev, "Failed to get regulators: %d\n", ret); + return ret; + } + + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(ctx->reset_gpio)) { + ret = PTR_ERR(ctx->reset_gpio); + dev_err(dev, "Failed to get reset-gpios: %d\n", ret); + return ret; + } + + ctx->dsi = dsi; + mipi_dsi_set_drvdata(dsi, ctx); + + dsi->lanes = 2; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST; + + drm_panel_init(&ctx->panel, dev, &s6e88a0_ams452ef01_panel_funcs, + DRM_MODE_CONNECTOR_DSI); + + ret = drm_panel_add(&ctx->panel); + if (ret < 0) { + dev_err(dev, "Failed to add panel: %d\n", ret); + return ret; + } + + ret = mipi_dsi_attach(dsi); + if (ret < 0) { + dev_err(dev, "Failed to attach to DSI host: %d\n", ret); + return ret; + } + + return 0; +} + +static int s6e88a0_ams452ef01_remove(struct mipi_dsi_device *dsi) +{ + struct s6e88a0_ams452ef01 *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); + + return 0; +} + +static const struct of_device_id s6e88a0_ams452ef01_of_match[] = { + { .compatible = "samsung,s6e88a0-ams452ef01" }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, s6e88a0_ams452ef01_of_match); + +static struct mipi_dsi_driver s6e88a0_ams452ef01_driver = { + .probe = s6e88a0_ams452ef01_probe, + .remove = s6e88a0_ams452ef01_remove, + .driver = { + .name = "panel-s6e88a0-ams452ef01", + .of_match_table = s6e88a0_ams452ef01_of_match, + }, +}; +module_mipi_dsi_driver(s6e88a0_ams452ef01_driver); + +MODULE_AUTHOR("Michael Srba Michael.Srba@seznam.cz"); +MODULE_DESCRIPTION("MIPI-DSI based Panel Driver for AMS452EF01 AMOLED LCD with a S6E88A0 controller"); +MODULE_LICENSE("GPL v2");
Hi Micahel.
Thanks for the quick feedback.
Please address checkpatch warnings.
be2b44c76320 (HEAD -> drm-misc-next) drm/panel: Add a driver for Samsung s6e88a0-ams452ef01 panel -:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
-:40: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #40: new file mode 100644
-:69: WARNING:LONG_LINE: line over 80 characters #69: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:25: +static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel)
-:74: WARNING:MACRO_WITH_FLOW_CONTROL: Macros with flow control statements should be avoided #74: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:30: +#define dsi_dcs_write_seq(dsi, seq...) do { \ + static const u8 d[] = { seq }; \ + int ret; \ + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \ + if (ret < 0) \ + return ret; \ + } while (0)
-:122: WARNING:LONG_LINE_COMMENT: line over 80 characters #122: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:78: + dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio
-:183: WARNING:LONG_LINE: line over 80 characters #183: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:139: + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
-:226: WARNING:LONG_LINE: line over 80 characters #226: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:182: +static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector)
total: 0 errors, 7 warnings, 0 checks, 308 lines checked
Use: dim checkpatch" if you have dim. Otherwise use checkpatch -q --emacs --strict --show-types
Ignore the warning "MACRO_WITH_FLOW_CONTROL" - as this seems legit in this use-case.
Sam
On Thu, Jan 30, 2020 at 06:11:28PM +0100, michael.srba@seznam.cz wrote:
From: Michael Srba Michael.Srba@seznam.cz
Signed-off-by: Michael Srba Michael.Srba@seznam.cz
Changes since v1: reorder includes; remove empty functions; fix after rebasing
drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile | 1 + .../panel/panel-samsung-s6e88a0-ams452ef01.c | 289 ++++++++++++++++++ 3 files changed, 296 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index ae44ac2ec106..0c7d61f32b0e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -275,6 +275,12 @@ config DRM_PANEL_SAMSUNG_S6E63M0 Say Y here if you want to enable support for Samsung S6E63M0 AMOLED LCD panel.
+config DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01
- tristate "Samsung AMS452EF01 panel with S6E88A0 DSI video mode controller"
- depends on OF
- select DRM_MIPI_DSI
- select VIDEOMODE_HELPERS
config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 7c4d3c581fd4..5b622fbe4014 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.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 +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01) += panel-samsung-s6e88a0-ams452ef01.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c new file mode 100644 index 000000000000..49d3f6d0f137 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c @@ -0,0 +1,289 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright (C) 2019, Michael Srba
+#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h>
+#include <video/mipi_display.h>
+#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h>
+struct s6e88a0_ams452ef01 {
- struct drm_panel panel;
- struct mipi_dsi_device *dsi;
- struct regulator_bulk_data supplies[2];
- struct gpio_desc *reset_gpio;
- bool prepared;
+};
+static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel) +{
- return container_of(panel, struct s6e88a0_ams452ef01, panel);
+}
+#define dsi_dcs_write_seq(dsi, seq...) do { \
static const u8 d[] = { seq }; \int ret; \ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \if (ret < 0) \return ret; \- } while (0)
+static void s6e88a0_ams452ef01_reset(struct s6e88a0_ams452ef01 *ctx) +{
- gpiod_set_value_cansleep(ctx->reset_gpio, 1);
- usleep_range(5000, 6000);
- gpiod_set_value_cansleep(ctx->reset_gpio, 0);
- usleep_range(1000, 2000);
- gpiod_set_value_cansleep(ctx->reset_gpio, 1);
- usleep_range(10000, 11000);
+}
+static int s6e88a0_ams452ef01_on(struct s6e88a0_ams452ef01 *ctx) +{
- struct mipi_dsi_device *dsi = ctx->dsi;
- struct device *dev = &dsi->dev;
- int ret;
- dsi->mode_flags |= MIPI_DSI_MODE_LPM;
- dsi_dcs_write_seq(dsi, 0xf0, 0x5a, 0x5a); // enable LEVEL2 commands
- dsi_dcs_write_seq(dsi, 0xcc, 0x4c); // set Pixel Clock Divider polarity
- 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);
- // set default brightness/gama
- dsi_dcs_write_seq(dsi, 0xca,
0x01, 0x00, 0x01, 0x00, 0x01, 0x00, // V255 RR,GG,BB0x80, 0x80, 0x80, // V203 R,G,B0x80, 0x80, 0x80, // V151 R,G,B0x80, 0x80, 0x80, // V87 R,G,B0x80, 0x80, 0x80, // V51 R,G,B0x80, 0x80, 0x80, // V35 R,G,B0x80, 0x80, 0x80, // V23 R,G,B0x80, 0x80, 0x80, // V11 R,G,B0x6b, 0x68, 0x71, // V3 R,G,B0x00, 0x00, 0x00); // V1 R,G,B- dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio
- dsi_dcs_write_seq(dsi, 0xb6, 0x2c, 0x0b); // set default elvss voltage
- dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, 0x00);
- dsi_dcs_write_seq(dsi, 0xf7, 0x03); // gamma/aor update
- dsi_dcs_write_seq(dsi, 0xf0, 0xa5, 0xa5); // disable LEVEL2 commands
- 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 s6e88a0_ams452ef01_off(struct s6e88a0_ams452ef01 *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(35);
- 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 s6e88a0_ams452ef01_prepare(struct drm_panel *panel) +{
- struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel);
- struct device *dev = &ctx->dsi->dev;
- int ret;
- if (ctx->prepared)
return 0;- ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- if (ret < 0) {
dev_err(dev, "Failed to enable regulators: %d\n", ret);return ret;- }
- s6e88a0_ams452ef01_reset(ctx);
- ret = s6e88a0_ams452ef01_on(ctx);
- if (ret < 0) {
dev_err(dev, "Failed to initialize panel: %d\n", ret);gpiod_set_value_cansleep(ctx->reset_gpio, 0);regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);return ret;- }
- ctx->prepared = true;
- return 0;
+}
+static int s6e88a0_ams452ef01_unprepare(struct drm_panel *panel) +{
- struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel);
- struct device *dev = &ctx->dsi->dev;
- int ret;
- if (!ctx->prepared)
return 0;- ret = s6e88a0_ams452ef01_off(ctx);
- if (ret < 0)
dev_err(dev, "Failed to un-initialize panel: %d\n", ret);- gpiod_set_value_cansleep(ctx->reset_gpio, 0);
- regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- ctx->prepared = false;
- return 0;
+}
+static const struct drm_display_mode s6e88a0_ams452ef01_mode = {
- .clock = (540 + 88 + 4 + 20) * (960 + 14 + 2 + 8) * 60 / 1000,
- .hdisplay = 540,
- .hsync_start = 540 + 88,
- .hsync_end = 540 + 88 + 4,
- .htotal = 540 + 88 + 4 + 20,
- .vdisplay = 960,
- .vsync_start = 960 + 14,
- .vsync_end = 960 + 14 + 2,
- .vtotal = 960 + 14 + 2 + 8,
- .vrefresh = 60,
- .width_mm = 56,
- .height_mm = 100,
+};
+static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector) +{
- struct drm_display_mode *mode;
- mode = drm_mode_duplicate(connector->dev, &s6e88a0_ams452ef01_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;
- drm_mode_probed_add(connector, mode);
- return 1;
+}
+static const struct drm_panel_funcs s6e88a0_ams452ef01_panel_funcs = {
- .unprepare = s6e88a0_ams452ef01_unprepare,
- .prepare = s6e88a0_ams452ef01_prepare,
- .get_modes = s6e88a0_ams452ef01_get_modes,
+};
+static int s6e88a0_ams452ef01_probe(struct mipi_dsi_device *dsi) +{
- struct device *dev = &dsi->dev;
- struct s6e88a0_ams452ef01 *ctx;
- int ret;
- ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
return -ENOMEM;- ctx->supplies[0].supply = "vdd3";
- ctx->supplies[1].supply = "vci";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
ctx->supplies);- if (ret < 0) {
dev_err(dev, "Failed to get regulators: %d\n", ret);return ret;- }
- ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(ctx->reset_gpio)) {
ret = PTR_ERR(ctx->reset_gpio);dev_err(dev, "Failed to get reset-gpios: %d\n", ret);return ret;- }
- ctx->dsi = dsi;
- mipi_dsi_set_drvdata(dsi, ctx);
- dsi->lanes = 2;
- dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
- drm_panel_init(&ctx->panel, dev, &s6e88a0_ams452ef01_panel_funcs,
DRM_MODE_CONNECTOR_DSI);- ret = drm_panel_add(&ctx->panel);
- if (ret < 0) {
dev_err(dev, "Failed to add panel: %d\n", ret);return ret;- }
- ret = mipi_dsi_attach(dsi);
- if (ret < 0) {
dev_err(dev, "Failed to attach to DSI host: %d\n", ret);return ret;- }
- return 0;
+}
+static int s6e88a0_ams452ef01_remove(struct mipi_dsi_device *dsi) +{
- struct s6e88a0_ams452ef01 *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);
- return 0;
+}
+static const struct of_device_id s6e88a0_ams452ef01_of_match[] = {
- { .compatible = "samsung,s6e88a0-ams452ef01" },
- { /* sentinel */ },
+}; +MODULE_DEVICE_TABLE(of, s6e88a0_ams452ef01_of_match);
+static struct mipi_dsi_driver s6e88a0_ams452ef01_driver = {
- .probe = s6e88a0_ams452ef01_probe,
- .remove = s6e88a0_ams452ef01_remove,
- .driver = {
.name = "panel-s6e88a0-ams452ef01",.of_match_table = s6e88a0_ams452ef01_of_match,- },
+}; +module_mipi_dsi_driver(s6e88a0_ams452ef01_driver);
+MODULE_AUTHOR("Michael Srba Michael.Srba@seznam.cz"); +MODULE_DESCRIPTION("MIPI-DSI based Panel Driver for AMS452EF01 AMOLED LCD with a S6E88A0 controller");
+MODULE_LICENSE("GPL v2");
2.24.0
Sending this with thunderbird, fingers crossed for no formatting issues
Hi, I've taken about two weeks to send zinitix v2, and haven't heard about that or v3 yet, so combined with how easy it was to address the issues, I didn't want to risk that was the reason :) I'm not sure how I should make the subject shorter, it's already pretty barebones, so if it has to be done, I assume you would have a suggestion on what is not as important as it seems to me? I'm also not sure if splitting the comment on line 78 to three lines (two seem to not suffice) wouldn't be even worse than leaving it like this? Or should I have it more to the left so that it fits on just two lines? Similarly, I'm not sure what the convention for splitting would be on line 25, since I saw the line breaks placed mostly after a whole function parameter. Actually, for all of the above-80 lines except the one that I missed when updating it today I don't really know how to handle them... except maybe line 139, I guess I missed that since it's off-by-one
On 30. 01. 20 19:28, Sam Ravnborg wrote:
Hi Micahel.
Thanks for the quick feedback.
Please address checkpatch warnings.
be2b44c76320 (HEAD -> drm-misc-next) drm/panel: Add a driver for Samsung s6e88a0-ams452ef01 panel -:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
-:40: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #40: new file mode 100644
-:69: WARNING:LONG_LINE: line over 80 characters #69: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:25: +static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel)
-:74: WARNING:MACRO_WITH_FLOW_CONTROL: Macros with flow control statements should be avoided #74: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:30: +#define dsi_dcs_write_seq(dsi, seq...) do { \
static const u8 d[] = { seq }; \int ret; \ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \if (ret < 0) \return ret; \- } while (0)
-:122: WARNING:LONG_LINE_COMMENT: line over 80 characters #122: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:78:
- dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio
-:183: WARNING:LONG_LINE: line over 80 characters #183: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:139:
regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);-:226: WARNING:LONG_LINE: line over 80 characters #226: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:182: +static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector)
total: 0 errors, 7 warnings, 0 checks, 308 lines checked
Use: dim checkpatch" if you have dim. Otherwise use checkpatch -q --emacs --strict --show-types
Ignore the warning "MACRO_WITH_FLOW_CONTROL" - as this seems legit in this use-case.
Sam
On Thu, Jan 30, 2020 at 06:11:28PM +0100, michael.srba@seznam.cz wrote:
From: Michael Srba Michael.Srba@seznam.cz
Signed-off-by: Michael Srba Michael.Srba@seznam.cz
Changes since v1: reorder includes; remove empty functions; fix after rebasing
drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile | 1 + .../panel/panel-samsung-s6e88a0-ams452ef01.c | 289 ++++++++++++++++++ 3 files changed, 296 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index ae44ac2ec106..0c7d61f32b0e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -275,6 +275,12 @@ config DRM_PANEL_SAMSUNG_S6E63M0 Say Y here if you want to enable support for Samsung S6E63M0 AMOLED LCD panel.
+config DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01
- tristate "Samsung AMS452EF01 panel with S6E88A0 DSI video mode controller"
- depends on OF
- select DRM_MIPI_DSI
- select VIDEOMODE_HELPERS
config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 7c4d3c581fd4..5b622fbe4014 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.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 +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01) += panel-samsung-s6e88a0-ams452ef01.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c new file mode 100644 index 000000000000..49d3f6d0f137 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c @@ -0,0 +1,289 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright (C) 2019, Michael Srba
+#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h>
+#include <video/mipi_display.h>
+#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h>
+struct s6e88a0_ams452ef01 {
- struct drm_panel panel;
- struct mipi_dsi_device *dsi;
- struct regulator_bulk_data supplies[2];
- struct gpio_desc *reset_gpio;
- bool prepared;
+};
+static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel) +{
- return container_of(panel, struct s6e88a0_ams452ef01, panel);
+}
+#define dsi_dcs_write_seq(dsi, seq...) do { \
static const u8 d[] = { seq }; \int ret; \ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \if (ret < 0) \return ret; \- } while (0)
+static void s6e88a0_ams452ef01_reset(struct s6e88a0_ams452ef01 *ctx) +{
- gpiod_set_value_cansleep(ctx->reset_gpio, 1);
- usleep_range(5000, 6000);
- gpiod_set_value_cansleep(ctx->reset_gpio, 0);
- usleep_range(1000, 2000);
- gpiod_set_value_cansleep(ctx->reset_gpio, 1);
- usleep_range(10000, 11000);
+}
+static int s6e88a0_ams452ef01_on(struct s6e88a0_ams452ef01 *ctx) +{
- struct mipi_dsi_device *dsi = ctx->dsi;
- struct device *dev = &dsi->dev;
- int ret;
- dsi->mode_flags |= MIPI_DSI_MODE_LPM;
- dsi_dcs_write_seq(dsi, 0xf0, 0x5a, 0x5a); // enable LEVEL2 commands
- dsi_dcs_write_seq(dsi, 0xcc, 0x4c); // set Pixel Clock Divider polarity
- 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);
- // set default brightness/gama
- dsi_dcs_write_seq(dsi, 0xca,
0x01, 0x00, 0x01, 0x00, 0x01, 0x00, // V255 RR,GG,BB0x80, 0x80, 0x80, // V203 R,G,B0x80, 0x80, 0x80, // V151 R,G,B0x80, 0x80, 0x80, // V87 R,G,B0x80, 0x80, 0x80, // V51 R,G,B0x80, 0x80, 0x80, // V35 R,G,B0x80, 0x80, 0x80, // V23 R,G,B0x80, 0x80, 0x80, // V11 R,G,B0x6b, 0x68, 0x71, // V3 R,G,B0x00, 0x00, 0x00); // V1 R,G,B- dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio
- dsi_dcs_write_seq(dsi, 0xb6, 0x2c, 0x0b); // set default elvss voltage
- dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, 0x00);
- dsi_dcs_write_seq(dsi, 0xf7, 0x03); // gamma/aor update
- dsi_dcs_write_seq(dsi, 0xf0, 0xa5, 0xa5); // disable LEVEL2 commands
- 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 s6e88a0_ams452ef01_off(struct s6e88a0_ams452ef01 *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(35);
- 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 s6e88a0_ams452ef01_prepare(struct drm_panel *panel) +{
- struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel);
- struct device *dev = &ctx->dsi->dev;
- int ret;
- if (ctx->prepared)
return 0;- ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- if (ret < 0) {
dev_err(dev, "Failed to enable regulators: %d\n", ret);return ret;- }
- s6e88a0_ams452ef01_reset(ctx);
- ret = s6e88a0_ams452ef01_on(ctx);
- if (ret < 0) {
dev_err(dev, "Failed to initialize panel: %d\n", ret);gpiod_set_value_cansleep(ctx->reset_gpio, 0);regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);return ret;- }
- ctx->prepared = true;
- return 0;
+}
+static int s6e88a0_ams452ef01_unprepare(struct drm_panel *panel) +{
- struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel);
- struct device *dev = &ctx->dsi->dev;
- int ret;
- if (!ctx->prepared)
return 0;- ret = s6e88a0_ams452ef01_off(ctx);
- if (ret < 0)
dev_err(dev, "Failed to un-initialize panel: %d\n", ret);- gpiod_set_value_cansleep(ctx->reset_gpio, 0);
- regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- ctx->prepared = false;
- return 0;
+}
+static const struct drm_display_mode s6e88a0_ams452ef01_mode = {
- .clock = (540 + 88 + 4 + 20) * (960 + 14 + 2 + 8) * 60 / 1000,
- .hdisplay = 540,
- .hsync_start = 540 + 88,
- .hsync_end = 540 + 88 + 4,
- .htotal = 540 + 88 + 4 + 20,
- .vdisplay = 960,
- .vsync_start = 960 + 14,
- .vsync_end = 960 + 14 + 2,
- .vtotal = 960 + 14 + 2 + 8,
- .vrefresh = 60,
- .width_mm = 56,
- .height_mm = 100,
+};
+static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector) +{
- struct drm_display_mode *mode;
- mode = drm_mode_duplicate(connector->dev, &s6e88a0_ams452ef01_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;
- drm_mode_probed_add(connector, mode);
- return 1;
+}
+static const struct drm_panel_funcs s6e88a0_ams452ef01_panel_funcs = {
- .unprepare = s6e88a0_ams452ef01_unprepare,
- .prepare = s6e88a0_ams452ef01_prepare,
- .get_modes = s6e88a0_ams452ef01_get_modes,
+};
+static int s6e88a0_ams452ef01_probe(struct mipi_dsi_device *dsi) +{
- struct device *dev = &dsi->dev;
- struct s6e88a0_ams452ef01 *ctx;
- int ret;
- ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
return -ENOMEM;- ctx->supplies[0].supply = "vdd3";
- ctx->supplies[1].supply = "vci";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
ctx->supplies);- if (ret < 0) {
dev_err(dev, "Failed to get regulators: %d\n", ret);return ret;- }
- ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(ctx->reset_gpio)) {
ret = PTR_ERR(ctx->reset_gpio);dev_err(dev, "Failed to get reset-gpios: %d\n", ret);return ret;- }
- ctx->dsi = dsi;
- mipi_dsi_set_drvdata(dsi, ctx);
- dsi->lanes = 2;
- dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
- drm_panel_init(&ctx->panel, dev, &s6e88a0_ams452ef01_panel_funcs,
DRM_MODE_CONNECTOR_DSI);- ret = drm_panel_add(&ctx->panel);
- if (ret < 0) {
dev_err(dev, "Failed to add panel: %d\n", ret);return ret;- }
- ret = mipi_dsi_attach(dsi);
- if (ret < 0) {
dev_err(dev, "Failed to attach to DSI host: %d\n", ret);return ret;- }
- return 0;
+}
+static int s6e88a0_ams452ef01_remove(struct mipi_dsi_device *dsi) +{
- struct s6e88a0_ams452ef01 *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);
- return 0;
+}
+static const struct of_device_id s6e88a0_ams452ef01_of_match[] = {
- { .compatible = "samsung,s6e88a0-ams452ef01" },
- { /* sentinel */ },
+}; +MODULE_DEVICE_TABLE(of, s6e88a0_ams452ef01_of_match);
+static struct mipi_dsi_driver s6e88a0_ams452ef01_driver = {
- .probe = s6e88a0_ams452ef01_probe,
- .remove = s6e88a0_ams452ef01_remove,
- .driver = {
.name = "panel-s6e88a0-ams452ef01",.of_match_table = s6e88a0_ams452ef01_of_match,- },
+}; +module_mipi_dsi_driver(s6e88a0_ams452ef01_driver);
+MODULE_AUTHOR("Michael Srba Michael.Srba@seznam.cz"); +MODULE_DESCRIPTION("MIPI-DSI based Panel Driver for AMS452EF01 AMOLED LCD with a S6E88A0 controller");
+MODULE_LICENSE("GPL v2");
2.24.0
Hi Michael.
On Thu, Jan 30, 2020 at 08:02:31PM +0100, Michael Srba wrote:
Sending this with thunderbird, fingers crossed for no formatting issues
Readable...
Hi, I've taken about two weeks to send zinitix v2, and haven't heard about that or v3 yet, so combined with how easy it was to address the issues, I didn't want to risk that was the reason :) I'm not sure how I should make the subject shorter, it's already pretty barebones, so if it has to be done, I assume you would have a suggestion on what is not as important as it seems to me? I'm also not sure if splitting the comment on line 78 to three lines (two seem to not suffice) wouldn't be even worse than leaving it like this? Or should I have it more to the left so that it fits on just two lines? Similarly, I'm not sure what the convention for splitting would be on line 25, since I saw the line breaks placed mostly after a whole function parameter. Actually, for all of the above-80 lines except the one that I missed when updating it today I don't really know how to handle them... except maybe line 139, I guess I missed that since it's off-by-one
Address what you think needs to be addressed. If I feel strong about some of the remaining warnings I will fix when applying. And let you know what I did if this is the case.
Sam
On 30. 01. 20 19:28, Sam Ravnborg wrote:
Hi Micahel.
Thanks for the quick feedback.
Please address checkpatch warnings.
be2b44c76320 (HEAD -> drm-misc-next) drm/panel: Add a driver for Samsung s6e88a0-ams452ef01 panel -:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
-:40: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #40: new file mode 100644
-:69: WARNING:LONG_LINE: line over 80 characters #69: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:25: +static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel)
-:74: WARNING:MACRO_WITH_FLOW_CONTROL: Macros with flow control statements should be avoided #74: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:30: +#define dsi_dcs_write_seq(dsi, seq...) do { \
static const u8 d[] = { seq }; \int ret; \ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \if (ret < 0) \return ret; \- } while (0)
-:122: WARNING:LONG_LINE_COMMENT: line over 80 characters #122: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:78:
- dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio
-:183: WARNING:LONG_LINE: line over 80 characters #183: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:139:
regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);-:226: WARNING:LONG_LINE: line over 80 characters #226: FILE: drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c:182: +static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector)
total: 0 errors, 7 warnings, 0 checks, 308 lines checked
Use: dim checkpatch" if you have dim. Otherwise use checkpatch -q --emacs --strict --show-types
Ignore the warning "MACRO_WITH_FLOW_CONTROL" - as this seems legit in this use-case.
Sam
On Thu, Jan 30, 2020 at 06:11:28PM +0100, michael.srba@seznam.cz wrote:
From: Michael Srba Michael.Srba@seznam.cz
Signed-off-by: Michael Srba Michael.Srba@seznam.cz
Changes since v1: reorder includes; remove empty functions; fix after rebasing
drivers/gpu/drm/panel/Kconfig | 6 + drivers/gpu/drm/panel/Makefile | 1 + .../panel/panel-samsung-s6e88a0-ams452ef01.c | 289 ++++++++++++++++++ 3 files changed, 296 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index ae44ac2ec106..0c7d61f32b0e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -275,6 +275,12 @@ config DRM_PANEL_SAMSUNG_S6E63M0 Say Y here if you want to enable support for Samsung S6E63M0 AMOLED LCD panel.
+config DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01
- tristate "Samsung AMS452EF01 panel with S6E88A0 DSI video mode controller"
- depends on OF
- select DRM_MIPI_DSI
- select VIDEOMODE_HELPERS
config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 7c4d3c581fd4..5b622fbe4014 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.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 +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01) += panel-samsung-s6e88a0-ams452ef01.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c new file mode 100644 index 000000000000..49d3f6d0f137 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-s6e88a0-ams452ef01.c @@ -0,0 +1,289 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright (C) 2019, Michael Srba
+#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h>
+#include <video/mipi_display.h>
+#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h>
+struct s6e88a0_ams452ef01 {
- struct drm_panel panel;
- struct mipi_dsi_device *dsi;
- struct regulator_bulk_data supplies[2];
- struct gpio_desc *reset_gpio;
- bool prepared;
+};
+static inline struct s6e88a0_ams452ef01 *to_s6e88a0_ams452ef01(struct drm_panel *panel) +{
- return container_of(panel, struct s6e88a0_ams452ef01, panel);
+}
+#define dsi_dcs_write_seq(dsi, seq...) do { \
static const u8 d[] = { seq }; \int ret; \ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \if (ret < 0) \return ret; \- } while (0)
+static void s6e88a0_ams452ef01_reset(struct s6e88a0_ams452ef01 *ctx) +{
- gpiod_set_value_cansleep(ctx->reset_gpio, 1);
- usleep_range(5000, 6000);
- gpiod_set_value_cansleep(ctx->reset_gpio, 0);
- usleep_range(1000, 2000);
- gpiod_set_value_cansleep(ctx->reset_gpio, 1);
- usleep_range(10000, 11000);
+}
+static int s6e88a0_ams452ef01_on(struct s6e88a0_ams452ef01 *ctx) +{
- struct mipi_dsi_device *dsi = ctx->dsi;
- struct device *dev = &dsi->dev;
- int ret;
- dsi->mode_flags |= MIPI_DSI_MODE_LPM;
- dsi_dcs_write_seq(dsi, 0xf0, 0x5a, 0x5a); // enable LEVEL2 commands
- dsi_dcs_write_seq(dsi, 0xcc, 0x4c); // set Pixel Clock Divider polarity
- 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);
- // set default brightness/gama
- dsi_dcs_write_seq(dsi, 0xca,
0x01, 0x00, 0x01, 0x00, 0x01, 0x00, // V255 RR,GG,BB0x80, 0x80, 0x80, // V203 R,G,B0x80, 0x80, 0x80, // V151 R,G,B0x80, 0x80, 0x80, // V87 R,G,B0x80, 0x80, 0x80, // V51 R,G,B0x80, 0x80, 0x80, // V35 R,G,B0x80, 0x80, 0x80, // V23 R,G,B0x80, 0x80, 0x80, // V11 R,G,B0x6b, 0x68, 0x71, // V3 R,G,B0x00, 0x00, 0x00); // V1 R,G,B- dsi_dcs_write_seq(dsi, 0xb2, 0x40, 0x0a, 0x17, 0x00, 0x0a); // set default Amoled Off Ratio
- dsi_dcs_write_seq(dsi, 0xb6, 0x2c, 0x0b); // set default elvss voltage
- dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, 0x00);
- dsi_dcs_write_seq(dsi, 0xf7, 0x03); // gamma/aor update
- dsi_dcs_write_seq(dsi, 0xf0, 0xa5, 0xa5); // disable LEVEL2 commands
- 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 s6e88a0_ams452ef01_off(struct s6e88a0_ams452ef01 *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(35);
- 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 s6e88a0_ams452ef01_prepare(struct drm_panel *panel) +{
- struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel);
- struct device *dev = &ctx->dsi->dev;
- int ret;
- if (ctx->prepared)
return 0;- ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- if (ret < 0) {
dev_err(dev, "Failed to enable regulators: %d\n", ret);return ret;- }
- s6e88a0_ams452ef01_reset(ctx);
- ret = s6e88a0_ams452ef01_on(ctx);
- if (ret < 0) {
dev_err(dev, "Failed to initialize panel: %d\n", ret);gpiod_set_value_cansleep(ctx->reset_gpio, 0);regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);return ret;- }
- ctx->prepared = true;
- return 0;
+}
+static int s6e88a0_ams452ef01_unprepare(struct drm_panel *panel) +{
- struct s6e88a0_ams452ef01 *ctx = to_s6e88a0_ams452ef01(panel);
- struct device *dev = &ctx->dsi->dev;
- int ret;
- if (!ctx->prepared)
return 0;- ret = s6e88a0_ams452ef01_off(ctx);
- if (ret < 0)
dev_err(dev, "Failed to un-initialize panel: %d\n", ret);- gpiod_set_value_cansleep(ctx->reset_gpio, 0);
- regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- ctx->prepared = false;
- return 0;
+}
+static const struct drm_display_mode s6e88a0_ams452ef01_mode = {
- .clock = (540 + 88 + 4 + 20) * (960 + 14 + 2 + 8) * 60 / 1000,
- .hdisplay = 540,
- .hsync_start = 540 + 88,
- .hsync_end = 540 + 88 + 4,
- .htotal = 540 + 88 + 4 + 20,
- .vdisplay = 960,
- .vsync_start = 960 + 14,
- .vsync_end = 960 + 14 + 2,
- .vtotal = 960 + 14 + 2 + 8,
- .vrefresh = 60,
- .width_mm = 56,
- .height_mm = 100,
+};
+static int s6e88a0_ams452ef01_get_modes(struct drm_panel *panel, struct drm_connector *connector) +{
- struct drm_display_mode *mode;
- mode = drm_mode_duplicate(connector->dev, &s6e88a0_ams452ef01_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;
- drm_mode_probed_add(connector, mode);
- return 1;
+}
+static const struct drm_panel_funcs s6e88a0_ams452ef01_panel_funcs = {
- .unprepare = s6e88a0_ams452ef01_unprepare,
- .prepare = s6e88a0_ams452ef01_prepare,
- .get_modes = s6e88a0_ams452ef01_get_modes,
+};
+static int s6e88a0_ams452ef01_probe(struct mipi_dsi_device *dsi) +{
- struct device *dev = &dsi->dev;
- struct s6e88a0_ams452ef01 *ctx;
- int ret;
- ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
return -ENOMEM;- ctx->supplies[0].supply = "vdd3";
- ctx->supplies[1].supply = "vci";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
ctx->supplies);- if (ret < 0) {
dev_err(dev, "Failed to get regulators: %d\n", ret);return ret;- }
- ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(ctx->reset_gpio)) {
ret = PTR_ERR(ctx->reset_gpio);dev_err(dev, "Failed to get reset-gpios: %d\n", ret);return ret;- }
- ctx->dsi = dsi;
- mipi_dsi_set_drvdata(dsi, ctx);
- dsi->lanes = 2;
- dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
- drm_panel_init(&ctx->panel, dev, &s6e88a0_ams452ef01_panel_funcs,
DRM_MODE_CONNECTOR_DSI);- ret = drm_panel_add(&ctx->panel);
- if (ret < 0) {
dev_err(dev, "Failed to add panel: %d\n", ret);return ret;- }
- ret = mipi_dsi_attach(dsi);
- if (ret < 0) {
dev_err(dev, "Failed to attach to DSI host: %d\n", ret);return ret;- }
- return 0;
+}
+static int s6e88a0_ams452ef01_remove(struct mipi_dsi_device *dsi) +{
- struct s6e88a0_ams452ef01 *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);
- return 0;
+}
+static const struct of_device_id s6e88a0_ams452ef01_of_match[] = {
- { .compatible = "samsung,s6e88a0-ams452ef01" },
- { /* sentinel */ },
+}; +MODULE_DEVICE_TABLE(of, s6e88a0_ams452ef01_of_match);
+static struct mipi_dsi_driver s6e88a0_ams452ef01_driver = {
- .probe = s6e88a0_ams452ef01_probe,
- .remove = s6e88a0_ams452ef01_remove,
- .driver = {
.name = "panel-s6e88a0-ams452ef01",.of_match_table = s6e88a0_ams452ef01_of_match,- },
+}; +module_mipi_dsi_driver(s6e88a0_ams452ef01_driver);
+MODULE_AUTHOR("Michael Srba Michael.Srba@seznam.cz"); +MODULE_DESCRIPTION("MIPI-DSI based Panel Driver for AMS452EF01 AMOLED LCD with a S6E88A0 controller");
+MODULE_LICENSE("GPL v2");
2.24.0
Hi Michael.
Thanks for the quick feedback and the conversion. There is a few things you need to improve as noted below.
Sam
On Thu, Jan 30, 2020 at 06:11:27PM +0100, michael.srba@seznam.cz wrote:
From: Michael Srba Michael.Srba@seznam.cz
This patch adds dts bindings for Samsung AMS452EF01 AMOLED panel, which makes use of their S6E88A0 controller.
Signed-off-by: Michael Srba Michael.Srba@seznam.cz
Hi, Thanks for the review. I believe I've fixed everything, and I tested that I get image on drm-next with these patches applied.
Changes since v1: use yaml format for the binding
.../panel/samsung,s6e88a0-ams452ef01.yaml | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml
diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml new file mode 100644 index 000000000000..3d9b480ec706 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e88a0-ams452ef01.yaml @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: GPL-2.0-only
For new bindings please use: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml#
Filename must match
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Samsung AMS452EF01 AMOLED panel with S6E88A0 video mode DSI controller
+maintainers:
- Michael Srba Michael.Srba@seznam.cz
+allOf:
- $ref: panel-common.yaml#
+properties:
- compatible:
- const: samsung,s6e88a0-ams452ef01
- reg: true
- reset-gpios: true
- vdd3-supply:
description: core voltage supply- vci-supply:
description: voltage supply for analog circuits- enforce-video-mode: true
enforce-video-mode is not referenced in the driver - is it relevant?
+required:
- compatible
- reg
- vdd3-supply
- vci-supply
- reset-gpios
+additionalProperties: false
+examples:
- |
- #include <dt-bindings/gpio/gpio.h>
- panel@0 {
reg = <0>;compatible = "samsung,s6e88a0-ams452ef01";vdd3-supply = <&pm8916_l17>;vci-supply = <®_vlcd_vci>;reset-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;- };
No tabs in yaml files. And fix indent so it matches. Closing '}' below 'p' in ports.
+...
Drop - not needed.
dri-devel@lists.freedesktop.org
-
Michael Srba -
michael.srba＠seznam.cz
-
Sam Ravnborg