Hi Josef. Thanks for updating the driver. On Mon, Jul 08, 2019 at 04:56:18PM +0200, Josef Lusticky wrote: > Add driver for Ilitek ILI9341 panels in parallel RGB mode > > Signed-off-by: Josef Lusticky <josef@xxxxxxxxxxx> > + */ > + > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/spi/spi.h> > + > +#include <video/mipi_display.h> > + > +#include <drm/drm_modes.h> > +#include <drm/drm_panel.h> > +#include <drm/drm_print.h> > +#include <drm/tinydrm/mipi-dbi.h> Good to see drivers that no longer uses drmP.h :-) > + > +/* ILI9341 extended register set (Vendor Command Set) */ > +#define ILI9341_IFMODE 0xB0 // clock polarity > +#define ILI9341_IFCTL 0xF6 // interface conrol > +#define ILI9341_PGAMCTRL 0xE0 // positive gamma control > +#define ILI9341_NGAMCTRL 0xE1 // negative gamma control > + > +#define ILI9341_MADCTL_MV BIT(5) > +#define ILI9341_MADCTL_MX BIT(6) > +#define ILI9341_MADCTL_MY BIT(7) > + > +/** > + * struct ili9341_config - the display specific configuration > + * @width_mm: physical panel width [mm] > + * @height_mm: physical panel height [mm] > + */ > +struct ili9341_config { > + u32 width_mm; > + u32 height_mm; > +}; > + > +struct ili9341 { > + struct drm_panel panel; > + struct mipi_dbi *mipi; > + const struct ili9341_config *conf; > +}; > + > +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) > +{ > + return container_of(panel, struct ili9341, panel); > +} > + > +static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) > +{ > + mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_OFF); > + mipi_dbi_command(ili->mipi, MIPI_DCS_ENTER_SLEEP_MODE); > + msleep(5); > + return 0; > +} > + > +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili) > +{ > + /* HW / SW Reset display and wait */ > + if (ili->mipi->reset) > + mipi_dbi_hw_reset(ili->mipi); > + > + mipi_dbi_command(ili->mipi, MIPI_DCS_SOFT_RESET); > + msleep(120); Consider a usleep_range here - to have the waiting a little more relaxed in the system. > + > + /* Polarity */ > + mipi_dbi_command(ili->mipi, ILI9341_IFMODE, 0xC0); > + > + /* Interface control */ > + mipi_dbi_command(ili->mipi, ILI9341_IFCTL, 0x09, 0x01, 0x26); > + > + /* Pixel format */ > + mipi_dbi_command(ili->mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_18BIT << 4); > + > + /* Gamma */ > + mipi_dbi_command(ili->mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01); > + mipi_dbi_command(ili->mipi, ILI9341_PGAMCTRL, > + 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1, > + 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00); > + mipi_dbi_command(ili->mipi, ILI9341_NGAMCTRL, > + 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1, > + 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f); > + > + /* Rotation */ > + mipi_dbi_command(ili->mipi, MIPI_DCS_SET_ADDRESS_MODE, ILI9341_MADCTL_MX); > + > + /* Exit sleep mode */ > + mipi_dbi_command(ili->mipi, MIPI_DCS_EXIT_SLEEP_MODE); > + msleep(120); > + > + mipi_dbi_command(ili->mipi, MIPI_DCS_SET_DISPLAY_ON); > + > + return 0; > +} > + > +static int ili9341_unprepare(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + > + return ili9341_deinit(panel, ili); > +} > + > +static int ili9341_prepare(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + int ret; > + > + ret = ili9341_init(panel, ili); > + if (ret < 0) > + ili9341_unprepare(panel); > + return ret; > +} > + > +static int ili9341_enable(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + > + return backlight_enable(ili->mipi->backlight); > +} > + > +static int ili9341_disable(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + > + return backlight_disable(ili->mipi->backlight); > +} > + > +static const struct drm_display_mode prgb_240x320_mode = { > + .clock = 6350, > + > + .hdisplay = 240, > + .hsync_start = 240 + 10, > + .hsync_end = 240 + 10 + 10, > + .htotal = 240 + 10 + 10 + 20, > + > + .vdisplay = 320, > + .vsync_start = 320 + 4, > + .vsync_end = 320 + 4 + 2, > + .vtotal = 320 + 4 + 2 + 2, > + > + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, > + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED > +}; > + > +static int ili9341_get_modes(struct drm_panel *panel) > +{ > + struct drm_connector *connector = panel->connector; > + struct ili9341 *ili = panel_to_ili9341(panel); > + struct drm_display_mode *mode; > + > + mode = drm_mode_duplicate(panel->drm, &prgb_240x320_mode); > + if (!mode) { > + DRM_DEV_ERROR(panel->drm->dev, "bad mode or failed to add mode\n"); > + return -ENOMEM; > + } > + > + drm_mode_set_name(mode); > + > + mode->width_mm = ili->conf->width_mm; > + mode->height_mm = ili->conf->height_mm; > + > + connector->display_info.width_mm = mode->width_mm; > + connector->display_info.height_mm = mode->height_mm; > + connector->display_info.bus_flags |= DRM_BUS_FLAG_DE_HIGH | > + DRM_BUS_FLAG_PIXDATA_POSEDGE | DRM_BUS_FLAG_SYNC_NEGEDGE; > + > + drm_mode_probed_add(connector, mode); > + > + return 1; /* Number of modes */ > +} > + > +static const struct drm_panel_funcs ili9341_drm_funcs = { > + .disable = ili9341_disable, > + .unprepare = ili9341_unprepare, > + .prepare = ili9341_prepare, > + .enable = ili9341_enable, > + .get_modes = ili9341_get_modes, > +}; > + > +static int ili9341_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct ili9341 *ili; > + struct mipi_dbi *mipi; > + struct gpio_desc *dc_gpio; > + int ret; > + > + mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); > + if (!mipi) > + return -ENOMEM; > + > + ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL); > + if (!ili) > + return -ENOMEM; > + > + ili->mipi = mipi; > + > + spi_set_drvdata(spi, ili); > + > + /* > + * Every new incarnation of this display must have a unique > + * data entry for the system in this driver. > + */ > + ili->conf = of_device_get_match_data(dev); > + if (!ili->conf) { > + DRM_DEV_ERROR(dev, "missing device configuration\n"); > + return -ENODEV; > + } > + > + ili->mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(ili->mipi->reset)) { > + DRM_DEV_ERROR(dev, "failed to get gpio 'reset'\n"); > + return PTR_ERR(ili->mipi->reset); > + } > + > + ili->mipi->backlight = devm_of_find_backlight(dev); > + if (IS_ERR(ili->mipi->backlight)) { > + DRM_DEV_ERROR(dev, "failed to get backlight\n"); > + return PTR_ERR(ili->mipi->backlight); > + } > + > + dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > + if (IS_ERR(dc_gpio)) { > + DRM_DEV_ERROR(dev, "failed to get gpio 'dc'\n"); > + return PTR_ERR(dc_gpio); > + } > + > + ret = mipi_dbi_spi_init(spi, ili->mipi, dc_gpio); > + if (ret) { > + DRM_DEV_ERROR(dev, "MIPI-DBI SPI setup failed\n"); > + return ret; > + } > + > + drm_panel_init(&ili->panel); > + ili->panel.dev = dev; > + ili->panel.funcs = &ili9341_drm_funcs; > + > + return drm_panel_add(&ili->panel); > +} > + > +static int ili9341_remove(struct spi_device *spi) > +{ > + struct ili9341 *ili = spi_get_drvdata(spi); > + > + drm_panel_remove(&ili->panel); > + > + ili9341_disable(&ili->panel); > + ili9341_unprepare(&ili->panel); > + > + return 0; > +} > + > +static const struct ili9341_config dt024ctft_data = { > + .width_mm = 37, > + .height_mm = 49, > +}; > + > +static const struct of_device_id ili9341_of_match[] = { > + { .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data }, > + { /* sentinel */ } > +}; If another display is supported then the drm_display_mode may not match. So the above may not prove enough in the future. for now it should be fine. > +MODULE_DEVICE_TABLE(of, ili9341_of_match); > + > +static struct spi_driver ili9341_driver = { > + .probe = ili9341_probe, > + .remove = ili9341_remove, > + .driver = { > + .name = "panel-ilitek-ili9341", > + .of_match_table = ili9341_of_match, > + }, > +}; > +module_spi_driver(ili9341_driver); > + > +MODULE_AUTHOR("Josef Lusticky <josef@xxxxxxxxxxx>"); > +MODULE_DESCRIPTION("ILI9341 LCD panel driver"); > +MODULE_LICENSE("GPL"); Looks good. Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx> Sam _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel