Message ID | 15b81a58eddd5d3fbfa418293cf1c817ef46423a.1548236066.git-series.maxime.ripard@bootlin.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/sun4i: dsi: Add burst mode support | expand |
Hi Maxime / Konstantin. Nice welstructured and small driver. Please see a few comments below Some of the comments in the following apply to a lot of the existing panel drivers as well. But lets see if we can get new drivers to be even better. Sam On Wed, Jan 23, 2019 at 04:54:24PM +0100, Maxime Ripard wrote: > From: Konstantin Sudakov <k.sudakov@integrasources.com> > > The Rondo RB070D30 panel is a MIPI-DSI panel based on a Fitipower EK79007 > controller and a 1024x600 panel. > > Signed-off-by: Konstantin Sudakov <k.sudakov@integrasources.com> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > drivers/gpu/drm/panel/Kconfig | 9 +- > drivers/gpu/drm/panel/Makefile | 1 +- > drivers/gpu/drm/panel/panel-rondo-rb070d30.c | 258 ++++++++++++++++++++- > 3 files changed, 268 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-rondo-rb070d30.c > > diff --git a/drivers/gpu/drm/panel/panel-rondo-rb070d30.c b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c > new file mode 100644 > index 000000000000..4f8e63f367b1 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c > @@ -0,0 +1,258 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018, Bridge Systems BV > + * Copyright (C) 2018, Bootlin > + * Copyright (C) 2017, Free Electrons > + * > + * This file based on panel-ilitek-ili9881c.c > + */ > + > +#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/drmP.h> > +#include <drm/drm_panel.h> Please do not use drmP.h in new drivers. We are trying to get rid of it. > + > +#include <video/mipi_display.h> > +#include <video/of_display_timing.h> > +#include <video/videomode.h> > + > +struct rb070d30_panel { > + struct drm_panel panel; > + struct mipi_dsi_device *dsi; > + struct backlight_device *backlight; > + struct regulator *supply; > + > + struct { > + struct gpio_desc *power; > + struct gpio_desc *reset; > + struct gpio_desc *updn; > + struct gpio_desc *shlr; > + } gpios; > +}; > + > +static inline struct rb070d30_panel *panel_to_rb070d30_panel(struct drm_panel *panel) > +{ > + return container_of(panel, struct rb070d30_panel, panel); > +} > + > +static int rb070d30_panel_prepare(struct drm_panel *panel) > +{ > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > + int ret; > + > + ret = regulator_enable(ctx->supply); > + if (ret < 0) { > + dev_err(&ctx->dsi->dev, "Failed to enable supply: %d\n", ret); Use DRM_DEV_ERROR(...) to have consistent error message for the drm drivers. This is general for the whole file. > + return ret; > + } > + > + /* Reset */ > + msleep(20); > + gpiod_set_value(ctx->gpios.power, 1); > + msleep(20); > + gpiod_set_value(ctx->gpios.reset, 1); > + msleep(20); > + return 0; > +} To verify the above pointer to a datasheet would be nice. > + > +static int rb070d30_panel_unprepare(struct drm_panel *panel) > +{ > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > + > + gpiod_set_value(ctx->gpios.power, 0); > + gpiod_set_value(ctx->gpios.reset, 0); > + regulator_disable(ctx->supply); > + > + return 0; > +} There is sometimes timing constrains after deassert reset.. The order is not strictly reverse of prepare - is that on purpose? > + > +static int rb070d30_panel_enable(struct drm_panel *panel) > +{ > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > + int ret; > + > + ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi); > + if (ret) > + return ret; > + > + ret = backlight_enable(ctx->backlight); > + if (ret) > + goto out; > + > + return 0; > + > +out: > + mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); > + return ret; > +} > + > +static int rb070d30_panel_disable(struct drm_panel *panel) > +{ > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > + > + backlight_disable(ctx->backlight); > + return mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); > +} > + > +/* Default timings */ > +static const struct drm_display_mode default_mode = { > + .clock = 51206, > + .hdisplay = 1024, > + .hsync_start = 1024 + 160, > + .hsync_end = 1024 + 160 + 80, > + .htotal = 1024 + 160 + 80 + 80, > + .vdisplay = 600, > + .vsync_start = 600 + 12, > + .vsync_end = 600 + 12 + 10, > + .vtotal = 600 + 12 + 10 + 13, > + .vrefresh = 60, > +}; height and width missing here. Seems better to add them here than hiding in code below. Is it on purpose that no flags are specified? > + > +static int rb070d30_panel_get_modes(struct drm_panel *panel) > +{ > + struct drm_connector *connector = panel->connector; > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > + struct drm_display_mode *mode; > + static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; > + > + mode = drm_mode_duplicate(panel->drm, &default_mode); > + if (!mode) { > + dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n", > + default_mode.hdisplay, default_mode.vdisplay, > + default_mode.vrefresh); Use" DRM_DEV_ERROR(&ctx->dsi->dev, "failed to add mode" DRM_MODE_FMT, DRM_MODE_ARG(mode)); > + return -EINVAL; > + } > + > + drm_mode_set_name(mode); > + > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > + drm_mode_probed_add(connector, mode); > + > + panel->connector->display_info.bpc = 8; > + panel->connector->display_info.width_mm = 154; > + panel->connector->display_info.height_mm = 85; See comment on height above. Same goes for bpc > + drm_display_info_set_bus_formats(&connector->display_info, > + &bus_format, 1); > + > + return 1; > +} > + > +static const struct drm_panel_funcs rb070d30_panel_funcs = { > + .get_modes = rb070d30_panel_get_modes, > + .prepare = rb070d30_panel_prepare, > + .enable = rb070d30_panel_enable, > + .disable = rb070d30_panel_disable, > + .unprepare = rb070d30_panel_unprepare, > +}; > + > +static int rb070d30_panel_dsi_probe(struct mipi_dsi_device *dsi) > +{ > + struct device_node *np; > + struct rb070d30_panel *ctx; > + int ret; > + > + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + ctx->supply = devm_regulator_get(&dsi->dev, "vcc-lcd"); > + if (IS_ERR(ctx->supply)) > + return PTR_ERR(ctx->supply); > + > + mipi_dsi_set_drvdata(dsi, ctx); > + ctx->dsi = dsi; > + > + drm_panel_init(&ctx->panel); > + ctx->panel.dev = &dsi->dev; > + ctx->panel.funcs = &rb070d30_panel_funcs; > + > + ctx->gpios.reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->gpios.reset)) { > + dev_err(&dsi->dev, "Couldn't get our reset GPIO\n"); > + return PTR_ERR(ctx->gpios.reset); > + } > + > + ctx->gpios.power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->gpios.power)) { > + dev_err(&dsi->dev, "Couldn't get our power GPIO\n"); > + return PTR_ERR(ctx->gpios.power); > + } > + > + ctx->gpios.updn = devm_gpiod_get(&dsi->dev, "updn", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->gpios.updn)) { > + dev_err(&dsi->dev, "Couldn't get our updn GPIO\n"); > + return PTR_ERR(ctx->gpios.updn); > + } This gpio is never used, it is only read from DT > + > + ctx->gpios.shlr = devm_gpiod_get(&dsi->dev, "shlr", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->gpios.shlr)) { > + dev_err(&dsi->dev, "Couldn't get our shlr GPIO\n"); > + return PTR_ERR(ctx->gpios.shlr); > + } This gpio is never used, it is only read from DT > + > + 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; > + } Use devm_of_find_backlight() > + > + ret = drm_panel_add(&ctx->panel); > + if (ret < 0) > + goto free_backlight; > + > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_LPM; > + dsi->format = MIPI_DSI_FMT_RGB888; > + dsi->lanes = 4; > + > + return mipi_dsi_attach(dsi); > + > +free_backlight: > + backlight_put(ctx->backlight); If devm_of_find_backlight() is used this can go. > + return ret; > +} > + > +static int rb070d30_panel_dsi_remove(struct mipi_dsi_device *dsi) > +{ > + struct rb070d30_panel *ctx = mipi_dsi_get_drvdata(dsi); > + > + mipi_dsi_detach(dsi); > + drm_panel_remove(&ctx->panel); > + backlight_put(ctx->backlight); If devm_of_find_backlight() is used this can go. > + > + return 0; > +} > + > +static const struct of_device_id rb070d30_panel_of_match[] = { > + { .compatible = "rondo,rb070d30" }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, rb070d30_panel_of_match); > + > +static struct mipi_dsi_driver rb070d30_panel_driver = { > + .probe = rb070d30_panel_dsi_probe, > + .remove = rb070d30_panel_dsi_remove, > + .driver = { > + .name = "panel-rondo-rb070d30", > + .of_match_table = rb070d30_panel_of_match, > + }, > +}; > +module_mipi_dsi_driver(rb070d30_panel_driver); > + > +MODULE_AUTHOR("Boris Brezillon <boris.brezillon@bootlin.com>"); > +MODULE_AUTHOR("Konstantin Sudakov <k.sudakov@integrasources.com>"); > +MODULE_DESCRIPTION("Rondo RB070D30 Panel Driver"); > +MODULE_LICENSE("GPL"); > -- > git-series 0.9.1 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Konstantin > >> + ctx->gpios.updn = devm_gpiod_get(&dsi->dev, "updn", GPIOD_OUT_LOW); > >> + if (IS_ERR(ctx->gpios.updn)) { > >> + dev_err(&dsi->dev, "Couldn't get our updn GPIO\n"); > >> + return PTR_ERR(ctx->gpios.updn); > >> + } > > This gpio is never used, it is only read from DT > The gpio is initialized with low state. The state may be inverted by DT. The same for the "shlr". > It is a vertical / horizontal inversion. Ohh, then it makes sense again. Consider adding a comment so this becomes more obvious Sam
Hi Sam, Thanks for the review, I'll address the points left out. On Sat, Jan 26, 2019 at 04:39:46PM +0100, Sam Ravnborg wrote: > > + return ret; > > + } > > + > > + /* Reset */ > > + msleep(20); > > + gpiod_set_value(ctx->gpios.power, 1); > > + msleep(20); > > + gpiod_set_value(ctx->gpios.reset, 1); > > + msleep(20); > > + return 0; > > +} > To verify the above pointer to a datasheet would be nice. Unfortunately, it's not publicly available :/ > > + > > +static int rb070d30_panel_unprepare(struct drm_panel *panel) > > +{ > > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > > + > > + gpiod_set_value(ctx->gpios.power, 0); > > + gpiod_set_value(ctx->gpios.reset, 0); > > + regulator_disable(ctx->supply); > > + > > + return 0; > > +} > > There is sometimes timing constrains after deassert reset.. > The order is not strictly reverse of prepare - is that on purpose? You're right about the order. I couldn't find any delay after a reset though in the datasheet. > > +/* Default timings */ > > +static const struct drm_display_mode default_mode = { > > + .clock = 51206, > > + .hdisplay = 1024, > > + .hsync_start = 1024 + 160, > > + .hsync_end = 1024 + 160 + 80, > > + .htotal = 1024 + 160 + 80 + 80, > > + .vdisplay = 600, > > + .vsync_start = 600 + 12, > > + .vsync_end = 600 + 12 + 10, > > + .vtotal = 600 + 12 + 10 + 13, > > + .vrefresh = 60, > > +}; > height and width missing here. Seems better to add them here than hiding in code below. > Is it on purpose that no flags are specified? > > > + > > +static int rb070d30_panel_get_modes(struct drm_panel *panel) > > +{ > > + struct drm_connector *connector = panel->connector; > > + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); > > + struct drm_display_mode *mode; > > + static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; > > + > > + mode = drm_mode_duplicate(panel->drm, &default_mode); > > + if (!mode) { > > + dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n", > > + default_mode.hdisplay, default_mode.vdisplay, > > + default_mode.vrefresh); > Use" > DRM_DEV_ERROR(&ctx->dsi->dev, > "failed to add mode" DRM_MODE_FMT, > DRM_MODE_ARG(mode)); > > + return -EINVAL; > > + } > > + > > + drm_mode_set_name(mode); > > + > > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > + drm_mode_probed_add(connector, mode); > > + > > + panel->connector->display_info.bpc = 8; > > + panel->connector->display_info.width_mm = 154; > > + panel->connector->display_info.height_mm = 85; > See comment on height above. > Same goes for bpc Sorry, I'm not sure to follow you here. bpc and height are both set? Thanks! Maxime
Hi Maxime. > > > + } > > > + > > > + drm_mode_set_name(mode); > > > + > > > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > > + drm_mode_probed_add(connector, mode); > > > + > > > + panel->connector->display_info.bpc = 8; > > > + panel->connector->display_info.width_mm = 154; > > > + panel->connector->display_info.height_mm = 85; > > See comment on height above. > > Same goes for bpc > > Sorry, I'm not sure to follow you here. bpc and height are both set? I assumed that if we had specified height and width in display_mode then we did not have to do it here. But I may be wrong. And for bpc I was plain wrong. Sam
Hi Sam, On Tue, Jan 29, 2019 at 04:48:33PM +0100, Sam Ravnborg wrote: > > > > + } > > > > + > > > > + drm_mode_set_name(mode); > > > > + > > > > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > > > + drm_mode_probed_add(connector, mode); > > > > + > > > > + panel->connector->display_info.bpc = 8; > > > > + panel->connector->display_info.width_mm = 154; > > > > + panel->connector->display_info.height_mm = 85; > > > See comment on height above. > > > Same goes for bpc > > > > Sorry, I'm not sure to follow you here. bpc and height are both set? > > I assumed that if we had specified height and width in display_mode > then we did not have to do it here. But I may be wrong. It looks like it's not done by the core, but panel-simple simply copies it, so I'll do it as well. Thanks for the suggestion! Maxime
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 3f3537719beb..3164fa824a63 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -138,6 +138,15 @@ config DRM_PANEL_RAYDIUM_RM68200 Say Y here if you want to enable support for Raydium RM68200 720x1280 DSI video mode panel. +config DRM_PANEL_RONDO_RB070D30 + tristate "Rondo Electronics RB070D30 panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for Rondo Electronics + RB070D30 1024x600 DSI panel. + config DRM_PANEL_SAMSUNG_S6D16D0 tristate "Samsung S6D16D0 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 4396658a7996..4fe4cf1bfdb5 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o +obj-$(CONFIG_DRM_PANEL_RONDO_RB070D30) += panel-rondo-rb070d30.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o diff --git a/drivers/gpu/drm/panel/panel-rondo-rb070d30.c b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c new file mode 100644 index 000000000000..4f8e63f367b1 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2018, Bridge Systems BV + * Copyright (C) 2018, Bootlin + * Copyright (C) 2017, Free Electrons + * + * This file based on panel-ilitek-ili9881c.c + */ + +#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/drmP.h> +#include <drm/drm_panel.h> + +#include <video/mipi_display.h> +#include <video/of_display_timing.h> +#include <video/videomode.h> + +struct rb070d30_panel { + struct drm_panel panel; + struct mipi_dsi_device *dsi; + struct backlight_device *backlight; + struct regulator *supply; + + struct { + struct gpio_desc *power; + struct gpio_desc *reset; + struct gpio_desc *updn; + struct gpio_desc *shlr; + } gpios; +}; + +static inline struct rb070d30_panel *panel_to_rb070d30_panel(struct drm_panel *panel) +{ + return container_of(panel, struct rb070d30_panel, panel); +} + +static int rb070d30_panel_prepare(struct drm_panel *panel) +{ + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); + int ret; + + ret = regulator_enable(ctx->supply); + if (ret < 0) { + dev_err(&ctx->dsi->dev, "Failed to enable supply: %d\n", ret); + return ret; + } + + /* Reset */ + msleep(20); + gpiod_set_value(ctx->gpios.power, 1); + msleep(20); + gpiod_set_value(ctx->gpios.reset, 1); + msleep(20); + return 0; +} + +static int rb070d30_panel_unprepare(struct drm_panel *panel) +{ + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); + + gpiod_set_value(ctx->gpios.power, 0); + gpiod_set_value(ctx->gpios.reset, 0); + regulator_disable(ctx->supply); + + return 0; +} + +static int rb070d30_panel_enable(struct drm_panel *panel) +{ + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); + int ret; + + ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi); + if (ret) + return ret; + + ret = backlight_enable(ctx->backlight); + if (ret) + goto out; + + return 0; + +out: + mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); + return ret; +} + +static int rb070d30_panel_disable(struct drm_panel *panel) +{ + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); + + backlight_disable(ctx->backlight); + return mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); +} + +/* Default timings */ +static const struct drm_display_mode default_mode = { + .clock = 51206, + .hdisplay = 1024, + .hsync_start = 1024 + 160, + .hsync_end = 1024 + 160 + 80, + .htotal = 1024 + 160 + 80 + 80, + .vdisplay = 600, + .vsync_start = 600 + 12, + .vsync_end = 600 + 12 + 10, + .vtotal = 600 + 12 + 10 + 13, + .vrefresh = 60, +}; + +static int rb070d30_panel_get_modes(struct drm_panel *panel) +{ + struct drm_connector *connector = panel->connector; + struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel); + struct drm_display_mode *mode; + static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; + + mode = drm_mode_duplicate(panel->drm, &default_mode); + if (!mode) { + dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n", + default_mode.hdisplay, default_mode.vdisplay, + default_mode.vrefresh); + return -EINVAL; + } + + drm_mode_set_name(mode); + + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; + drm_mode_probed_add(connector, mode); + + panel->connector->display_info.bpc = 8; + panel->connector->display_info.width_mm = 154; + panel->connector->display_info.height_mm = 85; + drm_display_info_set_bus_formats(&connector->display_info, + &bus_format, 1); + + return 1; +} + +static const struct drm_panel_funcs rb070d30_panel_funcs = { + .get_modes = rb070d30_panel_get_modes, + .prepare = rb070d30_panel_prepare, + .enable = rb070d30_panel_enable, + .disable = rb070d30_panel_disable, + .unprepare = rb070d30_panel_unprepare, +}; + +static int rb070d30_panel_dsi_probe(struct mipi_dsi_device *dsi) +{ + struct device_node *np; + struct rb070d30_panel *ctx; + int ret; + + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->supply = devm_regulator_get(&dsi->dev, "vcc-lcd"); + if (IS_ERR(ctx->supply)) + return PTR_ERR(ctx->supply); + + mipi_dsi_set_drvdata(dsi, ctx); + ctx->dsi = dsi; + + drm_panel_init(&ctx->panel); + ctx->panel.dev = &dsi->dev; + ctx->panel.funcs = &rb070d30_panel_funcs; + + ctx->gpios.reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(ctx->gpios.reset)) { + dev_err(&dsi->dev, "Couldn't get our reset GPIO\n"); + return PTR_ERR(ctx->gpios.reset); + } + + ctx->gpios.power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW); + if (IS_ERR(ctx->gpios.power)) { + dev_err(&dsi->dev, "Couldn't get our power GPIO\n"); + return PTR_ERR(ctx->gpios.power); + } + + ctx->gpios.updn = devm_gpiod_get(&dsi->dev, "updn", GPIOD_OUT_LOW); + if (IS_ERR(ctx->gpios.updn)) { + dev_err(&dsi->dev, "Couldn't get our updn GPIO\n"); + return PTR_ERR(ctx->gpios.updn); + } + + ctx->gpios.shlr = devm_gpiod_get(&dsi->dev, "shlr", GPIOD_OUT_LOW); + if (IS_ERR(ctx->gpios.shlr)) { + dev_err(&dsi->dev, "Couldn't get our shlr GPIO\n"); + return PTR_ERR(ctx->gpios.shlr); + } + + 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) + goto free_backlight; + + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_LPM; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->lanes = 4; + + return mipi_dsi_attach(dsi); + +free_backlight: + backlight_put(ctx->backlight); + return ret; +} + +static int rb070d30_panel_dsi_remove(struct mipi_dsi_device *dsi) +{ + struct rb070d30_panel *ctx = mipi_dsi_get_drvdata(dsi); + + mipi_dsi_detach(dsi); + drm_panel_remove(&ctx->panel); + backlight_put(ctx->backlight); + + return 0; +} + +static const struct of_device_id rb070d30_panel_of_match[] = { + { .compatible = "rondo,rb070d30" }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, rb070d30_panel_of_match); + +static struct mipi_dsi_driver rb070d30_panel_driver = { + .probe = rb070d30_panel_dsi_probe, + .remove = rb070d30_panel_dsi_remove, + .driver = { + .name = "panel-rondo-rb070d30", + .of_match_table = rb070d30_panel_of_match, + }, +}; +module_mipi_dsi_driver(rb070d30_panel_driver); + +MODULE_AUTHOR("Boris Brezillon <boris.brezillon@bootlin.com>"); +MODULE_AUTHOR("Konstantin Sudakov <k.sudakov@integrasources.com>"); +MODULE_DESCRIPTION("Rondo RB070D30 Panel Driver"); +MODULE_LICENSE("GPL");