| Message ID | 20181103100900.30313-10-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drm/sun4i: Allwinner MIPI-DSI Burst mode support | expand |
Hi Jagan. Reading through the driver triggered a few comments. Read and decide what is usefull. Sam > Add panel driver for it. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Note: init sequence is referenced from > https://github.com/longsleep/linux-pine64/blob/pine64-hacks-1.2/drivers/video/sunxi/disp2/disp/lcd/mb709_mipi.c This note should perferably be part of the commit message or maybe included in the code so it is easie to track back. > +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > +/* > + * Copyright (C) 2018 Amarula Solutions > + * Author: Jagan Teki <jagan@amarulasolutions.com> > + */ > + > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/errno.h> > +#include <linux/fb.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > + > +#include <linux/gpio/consumer.h> > +#include <linux/regulator/consumer.h> > + > +#include <drm/drm_mipi_dsi.h> > +#include <drm/drm_modes.h> > +#include <drm/drm_panel.h> > + > +#include <video/mipi_display.h> I cannot see this include is used anywhere in this .c file. The few MIPI_ constants originates from <drm/drm_mipi_dsi.h> and I think the include can be dropped. > +struct fy07024di26a30d { > + struct drm_panel panel; > + struct mipi_dsi_device *dsi; > + > + struct backlight_device *backlight; > + struct regulator *dvdd; > + struct regulator *avdd; > + struct gpio_desc *reset; > + > + bool is_enabled; > + bool is_prepared; > +}; Maybe bikeshedding a little here. But the use of names like fy07024di26a30d does not help readability. Just name it feiyang like for example we see in panel-innolux-p079zca.c where this drives is somehow inspired from. (Here the name is innolux). > + > +static inline struct fy07024di26a30d *panel_to_fy07024di26a30d(struct drm_panel *panel) > +{ > + return container_of(panel, struct fy07024di26a30d, panel); > +} > + > +struct fy07024di26a30d_init_cmd { > + size_t len; > + const char *data; > +}; > + > +#define FY07024DI26A30D(...) { \ > + .len = sizeof((char[]){__VA_ARGS__}), \ > + .data = (char[]){__VA_ARGS__} } > + > +static const struct fy07024di26a30d_init_cmd fy07024di26a30d_init_cmds[] = { > + FY07024DI26A30D(0x80, 0x58), > + FY07024DI26A30D(0x81, 0x47), > + FY07024DI26A30D(0x82, 0xD4), > + FY07024DI26A30D(0x83, 0x88), > + FY07024DI26A30D(0x84, 0xA9), > + FY07024DI26A30D(0x85, 0xC3), > + FY07024DI26A30D(0x86, 0x82), > +}; > + > +static int fy07024di26a30d_prepare(struct drm_panel *panel) > +{ > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > + struct mipi_dsi_device *dsi = ctx->dsi; > + unsigned int i; > + int ret; > + > + if (ctx->is_prepared) > + return 0; > + > + gpiod_set_value(ctx->reset, 1); > + msleep(50); > + > + gpiod_set_value(ctx->reset, 0); > + msleep(20); > + > + gpiod_set_value(ctx->reset, 1); > + msleep(200); > + > + for (i = 0; i < ARRAY_SIZE(fy07024di26a30d_init_cmds); i++) { > + const struct fy07024di26a30d_init_cmd *cmd = > + &fy07024di26a30d_init_cmds[i]; > + > + ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data, cmd->len); > + if (ret < 0) > + return ret; > + } > + > + ret = mipi_dsi_dcs_set_display_on(dsi); > + if (ret < 0) { > + dev_err(panel->dev, "failed to set display on: %d\n", ret); > + return ret; > + } General comment. Consider using DRM_DEV_ERROR(...) to be consistent with what is used in many other drm drivers. > +static int fy07024di26a30d_enable(struct drm_panel *panel) > +{ > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > + > + if (ctx->is_enabled) > + return 0; > + > + msleep(120); This msleep() looks unjustified, as no other statement preceed it. If prepare() requires a delay then maybe the delay should be there? > + > +static int fy07024di26a30d_unprepare(struct drm_panel *panel) > +{ > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > + int ret; > + > + if (!ctx->is_prepared) > + return 0; > + > + ret = mipi_dsi_dcs_set_display_off(ctx->dsi); > + if (ret < 0) > + dev_err(panel->dev, "failed to set display off: %d\n", ret); > + > + ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); > + if (ret < 0) > + dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret); > + > + msleep(100); > + > + regulator_disable(ctx->avdd); > + > + regulator_disable(ctx->dvdd); > + > + gpiod_set_value(ctx->reset, 0); > + > + gpiod_set_value(ctx->reset, 1); > + > + gpiod_set_value(ctx->reset, 0); In prepare() there are dealys around asserting reset and releasing again. Consider using a small helper function and be consistent with the delays. > + > + ctx->is_prepared = false; > + > + return 0; > +} > + > +static const struct drm_display_mode fy07024di26a30d_default_mode = { > + .clock = 55000, > + .vrefresh = 60, > + > + .hdisplay = 1024, > + .hsync_start = 1024 + 396, > + .hsync_end = 1024 + 396 + 20, > + .htotal = 1024 + 396 + 20 + 100, > + > + .vdisplay = 600, > + .vsync_start = 600 + 12, > + .vsync_end = 600 + 12 + 2, > + .vtotal = 600 + 12 + 2 + 21, > +}; Consider to assign .flags - if for nothign else then to document the default values. Many panels have started to do so in panel-simple.c for instance. This is also a proper place to specify the width_mm and height_mm, so the physical size is available. Assuming this is known. > + > +static int fy07024di26a30d_get_modes(struct drm_panel *panel) > +{ > + struct drm_connector *connector = panel->connector; > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > + struct drm_display_mode *mode; > + > + mode = drm_mode_duplicate(panel->drm, &fy07024di26a30d_default_mode); > + if (!mode) { > + dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n", > + fy07024di26a30d_default_mode.hdisplay, > + fy07024di26a30d_default_mode.vdisplay, > + fy07024di26a30d_default_mode.vrefresh); > + return -ENOMEM; > + } > + > + 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 fy07024di26a30d_funcs = { > + .disable = fy07024di26a30d_disable, > + .unprepare = fy07024di26a30d_unprepare, > + .prepare = fy07024di26a30d_prepare, > + .enable = fy07024di26a30d_enable, > + .get_modes = fy07024di26a30d_get_modes, > +}; > + > +static int fy07024di26a30d_dsi_probe(struct mipi_dsi_device *dsi) > +{ > + struct device_node *np; > + struct fy07024di26a30d *ctx; > + int ret; > + > + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + mipi_dsi_set_drvdata(dsi, ctx); > + ctx->dsi = dsi; > + > + drm_panel_init(&ctx->panel); > + ctx->panel.dev = &dsi->dev; > + ctx->panel.funcs = &fy07024di26a30d_funcs; > + > + ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd"); > + if (IS_ERR(ctx->dvdd)) { > + dev_err(&dsi->dev, "Couldn't get dvdd regulator\n"); > + return PTR_ERR(ctx->dvdd); > + } > + > + ctx->avdd = devm_regulator_get(&dsi->dev, "avdd"); > + if (IS_ERR(ctx->avdd)) { > + dev_err(&dsi->dev, "Couldn't get avdd regulator\n"); > + return PTR_ERR(ctx->avdd); > + } > + > + ret = regulator_enable(ctx->dvdd); > + if (ret) > + return ret; > + > + msleep(100); > + > + ret = regulator_enable(ctx->avdd); > + if (ret) > + return ret; > + > + msleep(5); The regulators are only enabled in the probe function. But disabled in the unprepare() function. Looking at other drivers is seems that the common way is to enable these in prepare() and disable these in unprepare(). Any particular reason the regulators are enabled in probe()? Maybe something to do with backlight? If so, then please put a comment. Also consider if there is somethign missing, as the panel cannot be used anymore after an unpreare() > + > + ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->reset)) { > + dev_err(&dsi->dev, "Couldn't get our reset GPIO\n"); > + return PTR_ERR(ctx->reset); > + } > + > + np = of_parse_phandle(dsi->dev.of_node, "backlight", 0); > + if (np) { > + ctx->backlight = of_find_backlight_by_node(np); > + of_node_put(np); > + > + if (!ctx->backlight) > + return -EPROBE_DEFER; > + } > + > + ret = drm_panel_add(&ctx->panel); > + if (ret < 0) > + return ret; > + > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST; > + dsi->format = MIPI_DSI_FMT_RGB888; > + dsi->lanes = 4; > + > + return mipi_dsi_attach(dsi); Several drivers undo the drm_panel_add() if mipi_dsi_attach() fails. I dunno if this is correct and when it may fail. > +MODULE_DESCRIPTION("Feiyang FY07024DI26A30-D MIPI-DSI LCD panel"); > +MODULE_LICENSE("GPL v2"); The SPDX tag mentioned MIT - so the above may not be correct.
On Mon, Nov 5, 2018 at 2:14 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Jagan. > > Reading through the driver triggered a few comments. > Read and decide what is usefull. > > Sam > > > Add panel driver for it. > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > Note: init sequence is referenced from > > https://github.com/longsleep/linux-pine64/blob/pine64-hacks-1.2/drivers/video/sunxi/disp2/disp/lcd/mb709_mipi.c > > This note should perferably be part of the commit message or maybe included > in the code so it is easie to track back. > > > > +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > > +/* > > + * Copyright (C) 2018 Amarula Solutions > > + * Author: Jagan Teki <jagan@amarulasolutions.com> > > + */ > > + > > +#include <linux/backlight.h> > > +#include <linux/delay.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/errno.h> > > +#include <linux/fb.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > + > > +#include <linux/gpio/consumer.h> > > +#include <linux/regulator/consumer.h> > > + > > +#include <drm/drm_mipi_dsi.h> > > +#include <drm/drm_modes.h> > > +#include <drm/drm_panel.h> > > + > > > > > +#include <video/mipi_display.h> > > I cannot see this include is used anywhere in this .c file. > The few MIPI_ constants originates from <drm/drm_mipi_dsi.h> > and I think the include can be dropped. > > > +struct fy07024di26a30d { > > + struct drm_panel panel; > > + struct mipi_dsi_device *dsi; > > + > > + struct backlight_device *backlight; > > + struct regulator *dvdd; > > + struct regulator *avdd; > > + struct gpio_desc *reset; > > + > > + bool is_enabled; > > + bool is_prepared; > > +}; > Maybe bikeshedding a little here. > But the use of names like fy07024di26a30d does not help readability. > Just name it feiyang like for example we see in panel-innolux-p079zca.c > where this drives is somehow inspired from. (Here the name is innolux). > > > > > + > > +static inline struct fy07024di26a30d *panel_to_fy07024di26a30d(struct drm_panel *panel) > > +{ > > + return container_of(panel, struct fy07024di26a30d, panel); > > +} > > + > > +struct fy07024di26a30d_init_cmd { > > + size_t len; > > + const char *data; > > +}; > > + > > +#define FY07024DI26A30D(...) { \ > > + .len = sizeof((char[]){__VA_ARGS__}), \ > > + .data = (char[]){__VA_ARGS__} } > > + > > +static const struct fy07024di26a30d_init_cmd fy07024di26a30d_init_cmds[] = { > > + FY07024DI26A30D(0x80, 0x58), > > + FY07024DI26A30D(0x81, 0x47), > > + FY07024DI26A30D(0x82, 0xD4), > > + FY07024DI26A30D(0x83, 0x88), > > + FY07024DI26A30D(0x84, 0xA9), > > + FY07024DI26A30D(0x85, 0xC3), > > + FY07024DI26A30D(0x86, 0x82), > > +}; > > + > > +static int fy07024di26a30d_prepare(struct drm_panel *panel) > > +{ > > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > > + struct mipi_dsi_device *dsi = ctx->dsi; > > + unsigned int i; > > + int ret; > > + > > + if (ctx->is_prepared) > > + return 0; > > + > > + gpiod_set_value(ctx->reset, 1); > > + msleep(50); > > + > > + gpiod_set_value(ctx->reset, 0); > > + msleep(20); > > + > > + gpiod_set_value(ctx->reset, 1); > > + msleep(200); > > + > > + for (i = 0; i < ARRAY_SIZE(fy07024di26a30d_init_cmds); i++) { > > + const struct fy07024di26a30d_init_cmd *cmd = > > + &fy07024di26a30d_init_cmds[i]; > > + > > + ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data, cmd->len); > > + if (ret < 0) > > + return ret; > > + } > > + > > + ret = mipi_dsi_dcs_set_display_on(dsi); > > + if (ret < 0) { > > + dev_err(panel->dev, "failed to set display on: %d\n", ret); > > + return ret; > > + } > General comment. > Consider using DRM_DEV_ERROR(...) to be consistent with > what is used in many other drm drivers. > > > > +static int fy07024di26a30d_enable(struct drm_panel *panel) > > +{ > > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > > + > > + if (ctx->is_enabled) > > + return 0; > > + > > + msleep(120); > This msleep() looks unjustified, as no other statement preceed it. > If prepare() requires a delay then maybe the delay should be there? I will use mipi_dsi_dcs_set_display_on in enable and this can be proper place other than prepare, since after prepare enable is the caller. > > > + > > +static int fy07024di26a30d_unprepare(struct drm_panel *panel) > > +{ > > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > > + int ret; > > + > > + if (!ctx->is_prepared) > > + return 0; > > + > > + ret = mipi_dsi_dcs_set_display_off(ctx->dsi); > > + if (ret < 0) > > + dev_err(panel->dev, "failed to set display off: %d\n", ret); > > + > > + ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); > > + if (ret < 0) > > + dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret); > > + > > + msleep(100); > > + > > + regulator_disable(ctx->avdd); > > + > > + regulator_disable(ctx->dvdd); > > + > > + gpiod_set_value(ctx->reset, 0); > > + > > + gpiod_set_value(ctx->reset, 1); > > + > > + gpiod_set_value(ctx->reset, 0); > > In prepare() there are dealys around asserting reset and releasing again. > Consider using a small helper function and be consistent with the > delays. Do you think exit calls also need delays? > > > + > > + ctx->is_prepared = false; > > + > > + return 0; > > +} > > + > > +static const struct drm_display_mode fy07024di26a30d_default_mode = { > > + .clock = 55000, > > + .vrefresh = 60, > > + > > + .hdisplay = 1024, > > + .hsync_start = 1024 + 396, > > + .hsync_end = 1024 + 396 + 20, > > + .htotal = 1024 + 396 + 20 + 100, > > + > > + .vdisplay = 600, > > + .vsync_start = 600 + 12, > > + .vsync_end = 600 + 12 + 2, > > + .vtotal = 600 + 12 + 2 + 21, > > +}; > Consider to assign .flags - if for nothign else then to document > the default values. > Many panels have started to do so in panel-simple.c for instance. I don't see .flags handle is done via bsp driver or with dts, ie reason I didn't use it. > > This is also a proper place to specify the width_mm and height_mm, > so the physical size is available. > Assuming this is known. Correct, I saw these active width, height is mentioned in datasheet. but the BSP tested dts simply initialized to 0 so I'm not using it. [2] > > > + > > +static int fy07024di26a30d_get_modes(struct drm_panel *panel) > > +{ > > + struct drm_connector *connector = panel->connector; > > + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); > > + struct drm_display_mode *mode; > > + > > + mode = drm_mode_duplicate(panel->drm, &fy07024di26a30d_default_mode); > > + if (!mode) { > > + dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n", > > + fy07024di26a30d_default_mode.hdisplay, > > + fy07024di26a30d_default_mode.vdisplay, > > + fy07024di26a30d_default_mode.vrefresh); > > + return -ENOMEM; > > + } > > + > > + 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 fy07024di26a30d_funcs = { > > + .disable = fy07024di26a30d_disable, > > + .unprepare = fy07024di26a30d_unprepare, > > + .prepare = fy07024di26a30d_prepare, > > + .enable = fy07024di26a30d_enable, > > + .get_modes = fy07024di26a30d_get_modes, > > +}; > > + > > +static int fy07024di26a30d_dsi_probe(struct mipi_dsi_device *dsi) > > +{ > > + struct device_node *np; > > + struct fy07024di26a30d *ctx; > > + int ret; > > + > > + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); > > + if (!ctx) > > + return -ENOMEM; > > + mipi_dsi_set_drvdata(dsi, ctx); > > + ctx->dsi = dsi; > > + > > + drm_panel_init(&ctx->panel); > > + ctx->panel.dev = &dsi->dev; > > + ctx->panel.funcs = &fy07024di26a30d_funcs; > > + > > + ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd"); > > + if (IS_ERR(ctx->dvdd)) { > > + dev_err(&dsi->dev, "Couldn't get dvdd regulator\n"); > > + return PTR_ERR(ctx->dvdd); > > + } > > + > > + ctx->avdd = devm_regulator_get(&dsi->dev, "avdd"); > > + if (IS_ERR(ctx->avdd)) { > > + dev_err(&dsi->dev, "Couldn't get avdd regulator\n"); > > + return PTR_ERR(ctx->avdd); > > + } > > + > > + ret = regulator_enable(ctx->dvdd); > > + if (ret) > > + return ret; > > + > > + msleep(100); > > + > > + ret = regulator_enable(ctx->avdd); > > + if (ret) > > + return ret; > > + > > + msleep(5); > The regulators are only enabled in the probe function. > But disabled in the unprepare() function. > > Looking at other drivers is seems that the common way is > to enable these in prepare() and disable these in unprepare(). > Any particular reason the regulators are enabled in probe()? > Maybe something to do with backlight? > If so, then please put a comment. > Also consider if there is somethign missing, as the panel cannot > be used anymore after an unpreare() Yes, ie true with many panels resides in mainline. but this panel is enabling regulators separately wrt reset. not done in inline. But it still work for me if I enable regulators in prepare, but not sure does it may any issues. > > > > + > > + ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); > > + if (IS_ERR(ctx->reset)) { > > + dev_err(&dsi->dev, "Couldn't get our reset GPIO\n"); > > + return PTR_ERR(ctx->reset); > > + } > > + > > + np = of_parse_phandle(dsi->dev.of_node, "backlight", 0); > > + if (np) { > > + ctx->backlight = of_find_backlight_by_node(np); > > + of_node_put(np); > > + > > + if (!ctx->backlight) > > + return -EPROBE_DEFER; > > + } > > + > > + ret = drm_panel_add(&ctx->panel); > > + if (ret < 0) > > + return ret; > > + > > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST; > > + dsi->format = MIPI_DSI_FMT_RGB888; > > + dsi->lanes = 4; > > + > > + return mipi_dsi_attach(dsi); > Several drivers undo the drm_panel_add() if mipi_dsi_attach() fails. > I dunno if this is correct and when it may fail. Yes it should be drm_panel_remove for fail case, will take care. > > > > +MODULE_DESCRIPTION("Feiyang FY07024DI26A30-D MIPI-DSI LCD panel"); > > > +MODULE_LICENSE("GPL v2"); > The SPDX tag mentioned MIT - so the above may not be correct. Correct, "GPL V2+ MIT" is fine? [2] https://github.com/armbian/build/blob/master/packages/blobs/sunxi/a64/pine64so.dts#L2182
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index d0d4e60f5153..bc70896fe43c 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -47,6 +47,15 @@ config DRM_PANEL_SIMPLE that it can be automatically turned off when the panel goes into a low power state. +config DRM_PANEL_FEIYANG_FY07024DI26A30D + tristate "Feiyang FY07024DI26A30-D MIPI-DSI LCD panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y if you want to enable support for panels based on the + Feiyang FY07024DI26A30-D MIPI-DSI interface. + config DRM_PANEL_ILITEK_IL9322 tristate "Ilitek ILI9322 320x240 QVGA panels" depends on OF && SPI diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 88011f06edb8..e23c017639c7 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o obj-$(CONFIG_DRM_PANEL_BANANAPI_S070WV20_ICN6211) += panel-bananapi-s070wv20-icn6211.o obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o +obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c new file mode 100644 index 000000000000..718631a72d8b --- /dev/null +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c @@ -0,0 +1,305 @@ +// SPDX-License-Identifier: (GPL-2.0+ or MIT) +/* + * Copyright (C) 2018 Amarula Solutions + * Author: Jagan Teki <jagan@amarulasolutions.com> + */ + +#include <linux/backlight.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/fb.h> +#include <linux/kernel.h> +#include <linux/module.h> + +#include <linux/gpio/consumer.h> +#include <linux/regulator/consumer.h> + +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> + +#include <video/mipi_display.h> + +struct fy07024di26a30d { + struct drm_panel panel; + struct mipi_dsi_device *dsi; + + struct backlight_device *backlight; + struct regulator *dvdd; + struct regulator *avdd; + struct gpio_desc *reset; + + bool is_enabled; + bool is_prepared; +}; + +static inline struct fy07024di26a30d *panel_to_fy07024di26a30d(struct drm_panel *panel) +{ + return container_of(panel, struct fy07024di26a30d, panel); +} + +struct fy07024di26a30d_init_cmd { + size_t len; + const char *data; +}; + +#define FY07024DI26A30D(...) { \ + .len = sizeof((char[]){__VA_ARGS__}), \ + .data = (char[]){__VA_ARGS__} } + +static const struct fy07024di26a30d_init_cmd fy07024di26a30d_init_cmds[] = { + FY07024DI26A30D(0x80, 0x58), + FY07024DI26A30D(0x81, 0x47), + FY07024DI26A30D(0x82, 0xD4), + FY07024DI26A30D(0x83, 0x88), + FY07024DI26A30D(0x84, 0xA9), + FY07024DI26A30D(0x85, 0xC3), + FY07024DI26A30D(0x86, 0x82), +}; + +static int fy07024di26a30d_prepare(struct drm_panel *panel) +{ + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); + struct mipi_dsi_device *dsi = ctx->dsi; + unsigned int i; + int ret; + + if (ctx->is_prepared) + return 0; + + gpiod_set_value(ctx->reset, 1); + msleep(50); + + gpiod_set_value(ctx->reset, 0); + msleep(20); + + gpiod_set_value(ctx->reset, 1); + msleep(200); + + for (i = 0; i < ARRAY_SIZE(fy07024di26a30d_init_cmds); i++) { + const struct fy07024di26a30d_init_cmd *cmd = + &fy07024di26a30d_init_cmds[i]; + + ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data, cmd->len); + if (ret < 0) + return ret; + } + + ret = mipi_dsi_dcs_set_display_on(dsi); + if (ret < 0) { + dev_err(panel->dev, "failed to set display on: %d\n", ret); + return ret; + } + + ctx->is_prepared = true; + + return 0; +} + +static int fy07024di26a30d_enable(struct drm_panel *panel) +{ + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); + + if (ctx->is_enabled) + return 0; + + msleep(120); + + backlight_enable(ctx->backlight); + ctx->is_enabled = true; + + return 0; +} + +static int fy07024di26a30d_disable(struct drm_panel *panel) +{ + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); + + if (!ctx->is_enabled) + return 0; + + backlight_disable(ctx->backlight); + ctx->is_enabled = false; + + return 0; +} + +static int fy07024di26a30d_unprepare(struct drm_panel *panel) +{ + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); + int ret; + + if (!ctx->is_prepared) + return 0; + + ret = mipi_dsi_dcs_set_display_off(ctx->dsi); + if (ret < 0) + dev_err(panel->dev, "failed to set display off: %d\n", ret); + + ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); + if (ret < 0) + dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret); + + msleep(100); + + regulator_disable(ctx->avdd); + + regulator_disable(ctx->dvdd); + + gpiod_set_value(ctx->reset, 0); + + gpiod_set_value(ctx->reset, 1); + + gpiod_set_value(ctx->reset, 0); + + ctx->is_prepared = false; + + return 0; +} + +static const struct drm_display_mode fy07024di26a30d_default_mode = { + .clock = 55000, + .vrefresh = 60, + + .hdisplay = 1024, + .hsync_start = 1024 + 396, + .hsync_end = 1024 + 396 + 20, + .htotal = 1024 + 396 + 20 + 100, + + .vdisplay = 600, + .vsync_start = 600 + 12, + .vsync_end = 600 + 12 + 2, + .vtotal = 600 + 12 + 2 + 21, +}; + +static int fy07024di26a30d_get_modes(struct drm_panel *panel) +{ + struct drm_connector *connector = panel->connector; + struct fy07024di26a30d *ctx = panel_to_fy07024di26a30d(panel); + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(panel->drm, &fy07024di26a30d_default_mode); + if (!mode) { + dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n", + fy07024di26a30d_default_mode.hdisplay, + fy07024di26a30d_default_mode.vdisplay, + fy07024di26a30d_default_mode.vrefresh); + return -ENOMEM; + } + + 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 fy07024di26a30d_funcs = { + .disable = fy07024di26a30d_disable, + .unprepare = fy07024di26a30d_unprepare, + .prepare = fy07024di26a30d_prepare, + .enable = fy07024di26a30d_enable, + .get_modes = fy07024di26a30d_get_modes, +}; + +static int fy07024di26a30d_dsi_probe(struct mipi_dsi_device *dsi) +{ + struct device_node *np; + struct fy07024di26a30d *ctx; + int ret; + + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + mipi_dsi_set_drvdata(dsi, ctx); + ctx->dsi = dsi; + + drm_panel_init(&ctx->panel); + ctx->panel.dev = &dsi->dev; + ctx->panel.funcs = &fy07024di26a30d_funcs; + + ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd"); + if (IS_ERR(ctx->dvdd)) { + dev_err(&dsi->dev, "Couldn't get dvdd regulator\n"); + return PTR_ERR(ctx->dvdd); + } + + ctx->avdd = devm_regulator_get(&dsi->dev, "avdd"); + if (IS_ERR(ctx->avdd)) { + dev_err(&dsi->dev, "Couldn't get avdd regulator\n"); + return PTR_ERR(ctx->avdd); + } + + ret = regulator_enable(ctx->dvdd); + if (ret) + return ret; + + msleep(100); + + ret = regulator_enable(ctx->avdd); + if (ret) + return ret; + + msleep(5); + + ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(ctx->reset)) { + dev_err(&dsi->dev, "Couldn't get our reset GPIO\n"); + return PTR_ERR(ctx->reset); + } + + np = of_parse_phandle(dsi->dev.of_node, "backlight", 0); + if (np) { + ctx->backlight = of_find_backlight_by_node(np); + of_node_put(np); + + if (!ctx->backlight) + return -EPROBE_DEFER; + } + + ret = drm_panel_add(&ctx->panel); + if (ret < 0) + return ret; + + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->lanes = 4; + + return mipi_dsi_attach(dsi); +} + +static int fy07024di26a30d_dsi_remove(struct mipi_dsi_device *dsi) +{ + struct fy07024di26a30d *ctx = mipi_dsi_get_drvdata(dsi); + + mipi_dsi_detach(dsi); + drm_panel_remove(&ctx->panel); + + if (ctx->backlight) + put_device(&ctx->backlight->dev); + + return 0; +} + +static const struct of_device_id fy07024di26a30d_of_match[] = { + { .compatible = "feiyang,fy07024di26a30d", }, + { } +}; +MODULE_DEVICE_TABLE(of, fy07024di26a30d_of_match); + +static struct mipi_dsi_driver fy07024di26a30d_driver = { + .probe = fy07024di26a30d_dsi_probe, + .remove = fy07024di26a30d_dsi_remove, + .driver = { + .name = "feiyang-fy07024di26a30d", + .of_match_table = fy07024di26a30d_of_match, + }, +}; +module_mipi_dsi_driver(fy07024di26a30d_driver); + +MODULE_AUTHOR("Jagan Teki <jagan@amarulasolutions.com>"); +MODULE_DESCRIPTION("Feiyang FY07024DI26A30-D MIPI-DSI LCD panel"); +MODULE_LICENSE("GPL v2");
Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel. Add panel driver for it. Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Note: init sequence is referenced from https://github.com/longsleep/linux-pine64/blob/pine64-hacks-1.2/drivers/video/sunxi/disp2/disp/lcd/mb709_mipi.c drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + .../drm/panel/panel-feiyang-fy07024di26a30d.c | 305 ++++++++++++++++++ 3 files changed, 315 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c