Hi Jagan. Thanks for this nice patch, a few comments below.
On Fri, Jan 25, 2019 at 12:13:13AM +0530, Jagan Teki wrote: > Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel. > > Add panel driver for it. > > Signed-off-by: Jagan Teki <[email protected]> > --- > Changes for v5: > - rebase on master > - adjust the hporch values to satisfy the refresh > Changes for v4: > - use simple structure for command init > - update proper comments on power, reset delay sequnce > - fix to use set_display_off in disable function > - move mode type to structure > - drop refres rate value, let drm compute > Changes for v2: > - new patch, derived from another dsi series > > MAINTAINERS | 6 + > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile | 1 + > .../drm/panel/panel-feiyang-fy07024di26a30d.c | 286 ++++++++++++++++++ > 4 files changed, 302 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index e3f785bad68b..5c8591eb5840 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4831,6 +4831,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/tve200/ > > +DRM DRIVER FOR FEIYANG FY07024DI26A30-D MIPI-DSI LCD PANELS > +M: Jagan Teki <[email protected]> > +S: Maintained > +F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > +F: > Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt > + > DRM DRIVER FOR ILITEK ILI9225 PANELS > M: David Lechner <[email protected]> > S: Maintained > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index 151ddf720b83..5304db7b7b55 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 54db0921f329..c88dacf9d439 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..f655647aeade > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > @@ -0,0 +1,286 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018 Amarula Solutions > + * Author: Jagan Teki <[email protected]> > + */ > + > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > + > +#include <linux/gpio/consumer.h> > +#include <linux/regulator/consumer.h> > + > +#include <drm/drmP.h> Please do not use drmP.h in new drivers. We are slowly gettting rid of it and in drm-misc.git the drmP.h file no longer contains anything (other than includes/forwards). > + gpiod_set_value(ctx->reset, 1); > + msleep(50); > + > + gpiod_set_value(ctx->reset, 0); > + /* T5 + T6 (avdd rise + video & logic signal rise) > + * T5 >= 10ms, 0 < T6 <= 10ms > + */ > + msleep(20); > + > + gpiod_set_value(ctx->reset, 1); Is it really necessary to do a 1 => 0 => 1 cycle here for reset? panel-simple do not need it. > + > +static int feiyang_dsi_probe(struct mipi_dsi_device *dsi) > +{ > + struct feiyang *ctx; > + int ret; > + > + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + mipi_dsi_set_drvdata(dsi, ctx); Add empty line atfer return -ENOMEM;? > + ctx->dsi = dsi; > + > + drm_panel_init(&ctx->panel); > + ctx->panel.dev = &dsi->dev; > + ctx->panel.funcs = &feiyang_funcs; > + > + ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd"); > + if (IS_ERR(ctx->dvdd)) { > + DRM_DEV_ERROR(&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)) { > + DRM_DEV_ERROR(&dsi->dev, "Couldn't get avdd regulator\n"); > + return PTR_ERR(ctx->avdd); > + } > + > + ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->reset)) { > + DRM_DEV_ERROR(&dsi->dev, "Couldn't get our reset GPIO\n"); > + return PTR_ERR(ctx->reset); > + } > + > + ctx->backlight = devm_of_find_backlight(&dsi->dev); > + if (IS_ERR(ctx->backlight)) > + return PTR_ERR(ctx->backlight); > + > + ret = drm_panel_add(&ctx->panel); > + if (ret < 0) > + goto put_backlight; > + > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST; > + dsi->format = MIPI_DSI_FMT_RGB888; > + dsi->lanes = 4; > + > + ret = mipi_dsi_attach(dsi); > + if (ret < 0) > + goto panel_remove; > + > + return ret; > + > +panel_remove: > + drm_panel_remove(&ctx->panel); > +put_backlight: > + if (ctx->backlight) > + put_device(&ctx->backlight->dev); As devm_of_find_backlight() is used the put_device() will be done for you. No need to do it here. I think the same is true for the panel. > + > + return ret; > +} > + > +static int feiyang_dsi_remove(struct mipi_dsi_device *dsi) > +{ > + struct feiyang *ctx = mipi_dsi_get_drvdata(dsi); > + > + mipi_dsi_detach(dsi); > + drm_panel_remove(&ctx->panel); > + > + if (ctx->backlight) > + put_device(&ctx->backlight->dev); Likewise, use of devm_of_find_backlight() will do this for you. Sam _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
