| Message ID | 20190729195526.13337-5-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mipi-dbi: Support panel drivers | expand |
Hi Noralf, see comments bellow. po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal: > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- > 1 file changed, 170 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > index a755f3e60111..dd333a642159 100644 > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > @@ -21,10 +21,13 @@ > #include <linux/gpio/consumer.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of_graph.h> > #include <linux/spi/spi.h> > > #include <video/mipi_display.h> > > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_drv.h> > #include <drm/drm_modes.h> > #include <drm/drm_panel.h> > #include <drm/drm_print.h> > @@ -32,10 +35,25 @@ > > /* ILI9341 extended register set (Vendor Command Set) */ > #define ILI9341_IFMODE 0xB0 // clock polarity > +#define ILI9341_FRMCTR1 0xb1 > +#define ILI9341_DISCTRL 0xb6 > +#define ILI9341_ETMOD 0xb7 > +#define ILI9341_PWCTRL1 0xc0 > +#define ILI9341_PWCTRL2 0xc1 > +#define ILI9341_VMCTRL1 0xc5 > +#define ILI9341_VMCTRL2 0xc7 > +#define ILI9341_PWCTRLA 0xcb > +#define ILI9341_PWCTRLB 0xcf > #define ILI9341_IFCTL 0xF6 // interface conrol > #define ILI9341_PGAMCTRL 0xE0 // positive gamma control > #define ILI9341_NGAMCTRL 0xE1 // negative gamma control > +#define ILI9341_DTCTRLA 0xe8 > +#define ILI9341_DTCTRLB 0xea > +#define ILI9341_PWRSEQ 0xed > +#define ILI9341_EN3GAM 0xf2 > +#define ILI9341_PUMPCTRL 0xf7 Keep the same format for values. > > +#define ILI9341_MADCTL_BGR BIT(3) > #define ILI9341_MADCTL_MV BIT(5) > #define ILI9341_MADCTL_MX BIT(6) > #define ILI9341_MADCTL_MY BIT(7) > @@ -44,15 +62,18 @@ > * struct ili9341_config - the display specific configuration > * @funcs: Panel functions > * @mode: Display mode > + * @command_mode_only: Panel only supports command mode Command_mode_only is a bit misleading name - when the MIPI_DBI interface is used for commands and image data then there are command and data transfers over the MIPI_DBI interface. So "command_mode_only" may confuse users with the usage of Data/Command pin/bit in MIPI_DBI transfers. Consider naming this like "serial_mode_only" or "serial_interface_only", or "prgb_mode_supported" and set to "true" for dt024ctft. > */ > struct ili9341_config { > const struct drm_panel_funcs *funcs; > const struct drm_display_mode *mode; > + bool command_mode_only; > }; > > struct ili9341 { > struct drm_panel panel; > - struct mipi_dbi dbi; > + struct mipi_dbi_dev dbidev; > + bool command_mode; Similarly to the comment above, this should be named "serial_input_mode" or (invertably) "prgb_input_mode". > struct backlight_device *backlight; > const struct ili9341_config *conf; > }; > @@ -64,7 +85,7 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) > > static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) > { > - struct mipi_dbi *dbi = &ili->dbi; > + struct mipi_dbi *dbi = &ili->dbidev.dbi; > > mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); > mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE); > @@ -74,7 +95,7 @@ static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) > > static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili) > { > - struct mipi_dbi *dbi = &ili->dbi; > + struct mipi_dbi *dbi = &ili->dbidev.dbi; > > /* HW / SW Reset display and wait */ > mipi_dbi_hw_reset(dbi); > @@ -164,6 +185,7 @@ static const struct drm_display_mode prgb_240x320_mode = { > .height_mm = 49, > }; > > +/* This is not used in command mode */ > static int ili9341_get_modes(struct drm_panel *panel) > { > struct drm_connector *connector = panel->connector; > @@ -201,19 +223,125 @@ static const struct ili9341_config dt024ctft_data = { > .mode = &prgb_240x320_mode, > }; > > +static int mi0283qt_prepare(struct drm_panel *panel) Consider naming this "ili9341_serial_prepare". > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + struct mipi_dbi *dbi = &ili->dbidev.dbi; > + u8 addr_mode; > + int ret; > + > + DRM_DEBUG_KMS("\n"); > + > + ret = mipi_dbi_poweron_conditional_reset(&ili->dbidev); > + if (ret < 0) > + return ret; > + if (ret == 1) > + goto out_enable; > + mipi_dbi_hw_reset(dbi); > + > + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); > + > + mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30); > + mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81); > + mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79); > + mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02); > + mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20); > + mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00); > + > + /* Power Control */ > + mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x26); > + mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11); > + /* VCOM */ > + mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e); > + mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe); > + > + /* Memory Access Control */ > + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT); > + > + /* Frame Rate */ > + mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b); > + > + /* Gamma */ > + mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08); > + mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01); > + mipi_dbi_command(dbi, ILI9341_PGAMCTRL, > + 0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87, > + 0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00); > + mipi_dbi_command(dbi, ILI9341_NGAMCTRL, > + 0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78, > + 0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f); > + > + /* DDRAM */ > + mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07); > + > + /* Display */ > + mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00); > + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); > + msleep(100); > + > + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON); > + msleep(100); > + > +out_enable: > + /* The PiTFT (ili9340) has a hardware reset circuit that > + * resets only on power-on and not on each reboot through > + * a gpio like the rpi-display does. > + * As a result, we need to always apply the rotation value > + * regardless of the display "on/off" state. > + */ > + switch (ili->dbidev.rotation) { > + default: > + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY | > + ILI9341_MADCTL_MX; > + break; > + case 90: > + addr_mode = ILI9341_MADCTL_MY; > + break; > + case 180: > + addr_mode = ILI9341_MADCTL_MV; > + break; > + case 270: > + addr_mode = ILI9341_MADCTL_MX; > + break; > + } > + addr_mode |= ILI9341_MADCTL_BGR; > + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); > + > + return 0; > +} > + > +static const struct drm_panel_funcs mi0283qt_drm_funcs = { > + .disable = ili9341_disable, > + .unprepare = ili9341_unprepare, > + .prepare = mi0283qt_prepare, > + .enable = ili9341_enable, > + .get_modes = ili9341_get_modes, > +}; > + > +static const struct drm_display_mode mi0283qt_mode = { > + DRM_SIMPLE_MODE(320, 240, 58, 43), > +}; > + > +static const struct ili9341_config mi0283qt_data = { > + .funcs = &mi0283qt_drm_funcs, > + .mode = &mi0283qt_mode, > + .command_mode_only = true, > +}; > + > static int ili9341_probe(struct spi_device *spi) > { > struct device *dev = &spi->dev; > struct ili9341 *ili; > struct mipi_dbi *dbi; > struct gpio_desc *dc_gpio; > + struct device_node *port; > int ret; > > - ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL); > + ili = kzalloc(sizeof(*ili), GFP_KERNEL); > if (!ili) > return -ENOMEM; > > - dbi = &ili->dbi; > + dbi = &ili->dbidev.dbi; > > spi_set_drvdata(spi, ili); > > @@ -255,23 +383,55 @@ static int ili9341_probe(struct spi_device *spi) > ili->panel.dev = dev; > ili->panel.funcs = ili->conf->funcs; > > - return drm_panel_add(&ili->panel); > + port = of_get_child_by_name(dev->of_node, "port"); > + if (port) > + of_node_put(port); > + else > + ili->command_mode = true; > + > + if (ili->conf->command_mode_only) > + ili->command_mode = true; > + > + if (ili->command_mode) > + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, ili->conf->mode); > + else > + ret = drm_panel_add(&ili->panel); > + > + return ret; > } > > static int ili9341_remove(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_panel_remove(&ili->panel); > + if (ili->command_mode) { > + struct drm_device *drm = &ili->dbidev.drm; > > - ili9341_disable(&ili->panel); > - ili9341_unprepare(&ili->panel); > + drm_dev_unplug(drm); > + drm_atomic_helper_shutdown(drm); > + } else { > + drm_panel_remove(&ili->panel); > + > + ili9341_disable(&ili->panel); > + ili9341_unprepare(&ili->panel); > + kfree(ili); > + } > > return 0; > } > > +static void ili9341_shutdown(struct spi_device *spi) > +{ > + struct ili9341 *ili = spi_get_drvdata(spi); > + > + if (ili->command_mode) > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > +} > + > static const struct of_device_id ili9341_of_match[] = { > { .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data }, > +/* { .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, */ > + { .compatible = "mi,mi0283qt", .data = &mi0283qt_data }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, ili9341_of_match); > @@ -279,6 +439,7 @@ MODULE_DEVICE_TABLE(of, ili9341_of_match); > static struct spi_driver ili9341_driver = { > .probe = ili9341_probe, > .remove = ili9341_remove, > + .shutdown = ili9341_shutdown, > .driver = { > .name = "panel-ilitek-ili9341", > .of_match_table = ili9341_of_match, > -- > 2.20.1 >
Den 30.07.2019 10.34, skrev Josef Luštický: > Hi Noralf, > see comments bellow. > > po 29. 7. 2019 v 21:55 odesílatel Noralf Trønnes <noralf@tronnes.org> napsal: >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- >> 1 file changed, 170 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> index a755f3e60111..dd333a642159 100644 >> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c <snip> >> @@ -44,15 +62,18 @@ >> * struct ili9341_config - the display specific configuration >> * @funcs: Panel functions >> * @mode: Display mode >> + * @command_mode_only: Panel only supports command mode > Command_mode_only is a bit misleading name - when the MIPI_DBI > interface is used for commands and > image data then there are command and data transfers over the MIPI_DBI > interface. > So "command_mode_only" may confuse users with the usage of > Data/Command pin/bit in MIPI_DBI transfers. > Consider naming this like "serial_mode_only" or "serial_interface_only", or > "prgb_mode_supported" and set to "true" for dt024ctft. > The reason for choosing this name is that the pixels are uploaded using a MIPI DCS command. Serial can be confusing since we have MIPI DSI which is serial, and MIPI DBI can be both serial and parallel. The MIPI interface name for the rgb pixel interface is DPI. We could flip the bool and call it dpi_mode. >> */ >> struct ili9341_config { >> const struct drm_panel_funcs *funcs; >> const struct drm_display_mode *mode; >> + bool command_mode_only; >> }; >> >> struct ili9341 { >> struct drm_panel panel; >> - struct mipi_dbi dbi; >> + struct mipi_dbi_dev dbidev; >> + bool command_mode; > Similarly to the comment above, this should be named > "serial_input_mode" or (invertably) "prgb_input_mode". > >> struct backlight_device *backlight; >> const struct ili9341_config *conf; >> }; >> @@ -64,7 +85,7 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) >> >> static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) >> { >> - struct mipi_dbi *dbi = &ili->dbi; >> + struct mipi_dbi *dbi = &ili->dbidev.dbi; >> >> mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); >> mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE); >> @@ -74,7 +95,7 @@ static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) >> >> static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili) >> { >> - struct mipi_dbi *dbi = &ili->dbi; >> + struct mipi_dbi *dbi = &ili->dbidev.dbi; >> >> /* HW / SW Reset display and wait */ >> mipi_dbi_hw_reset(dbi); >> @@ -164,6 +185,7 @@ static const struct drm_display_mode prgb_240x320_mode = { >> .height_mm = 49, >> }; >> >> +/* This is not used in command mode */ >> static int ili9341_get_modes(struct drm_panel *panel) >> { >> struct drm_connector *connector = panel->connector; >> @@ -201,19 +223,125 @@ static const struct ili9341_config dt024ctft_data = { >> .mode = &prgb_240x320_mode, >> }; >> >> +static int mi0283qt_prepare(struct drm_panel *panel) > Consider naming this "ili9341_serial_prepare". > This is the controller setup function for the MI0283QT panel, hence its name. It's not a generic controller function. Noralf. >> +{ >> + struct ili9341 *ili = panel_to_ili9341(panel); >> + struct mipi_dbi *dbi = &ili->dbidev.dbi; >> + u8 addr_mode; >> + int ret; >> + >> + DRM_DEBUG_KMS("\n"); >> + >> + ret = mipi_dbi_poweron_conditional_reset(&ili->dbidev); >> + if (ret < 0) >> + return ret; >> + if (ret == 1) >> + goto out_enable; >> + mipi_dbi_hw_reset(dbi); >> + >> + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); >> + >> + mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30); >> + mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81); >> + mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79); >> + mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02); >> + mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20); >> + mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00); >> + >> + /* Power Control */ >> + mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x26); >> + mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11); >> + /* VCOM */ >> + mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e); >> + mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe); >> + >> + /* Memory Access Control */ >> + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT); >> + >> + /* Frame Rate */ >> + mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b); >> + >> + /* Gamma */ >> + mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08); >> + mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01); >> + mipi_dbi_command(dbi, ILI9341_PGAMCTRL, >> + 0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87, >> + 0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00); >> + mipi_dbi_command(dbi, ILI9341_NGAMCTRL, >> + 0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78, >> + 0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f); >> + >> + /* DDRAM */ >> + mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07); >> + >> + /* Display */ >> + mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00); >> + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); >> + msleep(100); >> + >> + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON); >> + msleep(100); >> + >> +out_enable: >> + /* The PiTFT (ili9340) has a hardware reset circuit that >> + * resets only on power-on and not on each reboot through >> + * a gpio like the rpi-display does. >> + * As a result, we need to always apply the rotation value >> + * regardless of the display "on/off" state. >> + */ >> + switch (ili->dbidev.rotation) { >> + default: >> + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY | >> + ILI9341_MADCTL_MX; >> + break; >> + case 90: >> + addr_mode = ILI9341_MADCTL_MY; >> + break; >> + case 180: >> + addr_mode = ILI9341_MADCTL_MV; >> + break; >> + case 270: >> + addr_mode = ILI9341_MADCTL_MX; >> + break; >> + } >> + addr_mode |= ILI9341_MADCTL_BGR; >> + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); >> + >> + return 0; >> +} >> + >> +static const struct drm_panel_funcs mi0283qt_drm_funcs = { >> + .disable = ili9341_disable, >> + .unprepare = ili9341_unprepare, >> + .prepare = mi0283qt_prepare, >> + .enable = ili9341_enable, >> + .get_modes = ili9341_get_modes, >> +}; >> + >> +static const struct drm_display_mode mi0283qt_mode = { >> + DRM_SIMPLE_MODE(320, 240, 58, 43), >> +}; >> + >> +static const struct ili9341_config mi0283qt_data = { >> + .funcs = &mi0283qt_drm_funcs, >> + .mode = &mi0283qt_mode, >> + .command_mode_only = true, >> +}; >> + >> static int ili9341_probe(struct spi_device *spi) >> { >> struct device *dev = &spi->dev; >> struct ili9341 *ili; >> struct mipi_dbi *dbi; >> struct gpio_desc *dc_gpio; >> + struct device_node *port; >> int ret; >> >> - ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL); >> + ili = kzalloc(sizeof(*ili), GFP_KERNEL); >> if (!ili) >> return -ENOMEM; >> >> - dbi = &ili->dbi; >> + dbi = &ili->dbidev.dbi; >> >> spi_set_drvdata(spi, ili); >> >> @@ -255,23 +383,55 @@ static int ili9341_probe(struct spi_device *spi) >> ili->panel.dev = dev; >> ili->panel.funcs = ili->conf->funcs; >> >> - return drm_panel_add(&ili->panel); >> + port = of_get_child_by_name(dev->of_node, "port"); >> + if (port) >> + of_node_put(port); >> + else >> + ili->command_mode = true; >> + >> + if (ili->conf->command_mode_only) >> + ili->command_mode = true; >> + >> + if (ili->command_mode) >> + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, ili->conf->mode); >> + else >> + ret = drm_panel_add(&ili->panel); >> + >> + return ret; >> } >> >> static int ili9341_remove(struct spi_device *spi) >> { >> struct ili9341 *ili = spi_get_drvdata(spi); >> >> - drm_panel_remove(&ili->panel); >> + if (ili->command_mode) { >> + struct drm_device *drm = &ili->dbidev.drm; >> >> - ili9341_disable(&ili->panel); >> - ili9341_unprepare(&ili->panel); >> + drm_dev_unplug(drm); >> + drm_atomic_helper_shutdown(drm); >> + } else { >> + drm_panel_remove(&ili->panel); >> + >> + ili9341_disable(&ili->panel); >> + ili9341_unprepare(&ili->panel); >> + kfree(ili); >> + } >> >> return 0; >> } >> >> +static void ili9341_shutdown(struct spi_device *spi) >> +{ >> + struct ili9341 *ili = spi_get_drvdata(spi); >> + >> + if (ili->command_mode) >> + drm_atomic_helper_shutdown(&ili->dbidev.drm); >> +} >> + >> static const struct of_device_id ili9341_of_match[] = { >> { .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data }, >> +/* { .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, */ >> + { .compatible = "mi,mi0283qt", .data = &mi0283qt_data }, >> { /* sentinel */ } >> }; >> MODULE_DEVICE_TABLE(of, ili9341_of_match); >> @@ -279,6 +439,7 @@ MODULE_DEVICE_TABLE(of, ili9341_of_match); >> static struct spi_driver ili9341_driver = { >> .probe = ili9341_probe, >> .remove = ili9341_remove, >> + .shutdown = ili9341_shutdown, >> .driver = { >> .name = "panel-ilitek-ili9341", >> .of_match_table = ili9341_of_match, >> -- >> 2.20.1 >> >
Den 29.07.2019 21.55, skrev Noralf Trønnes: > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- > 1 file changed, 170 insertions(+), 9 deletions(-) > I have realised that this will change the DRM driver name from mi0283qt to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] support. I haven't tried it, but this is the first thing that gives this driver any real advantage over its fbdev counterpart in drivers/staging/fbtft, so I don't want to loose that. So even if MIPI DBI panel support is implemented in some form, I will wait with converting mi0283qt until someone has updated the kmsro driver. Noralf. [1] https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/winsys/kmsro/drm https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/targets/dri/target.c
On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote: > > > > Den 29.07.2019 21.55, skrev Noralf Trønnes: > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > --- > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- > > 1 file changed, 170 insertions(+), 9 deletions(-) > > > > I have realised that this will change the DRM driver name from mi0283qt > to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] > support. I haven't tried it, but this is the first thing that gives this > driver any real advantage over its fbdev counterpart in > drivers/staging/fbtft, so I don't want to loose that. > So even if MIPI DBI panel support is implemented in some form, I will > wait with converting mi0283qt until someone has updated the kmsro driver. Why does it change? You should be able to stuff whatever you feel like into the drm driver name, this doesn't have to match either your platform/spi/whatever driver name nor the module option. -Daniel > Noralf. > > [1] > https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/winsys/kmsro/drm > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/targets/dri/target.c
Den 30.07.2019 17.22, skrev Daniel Vetter: > On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote: >> >> >> >> Den 29.07.2019 21.55, skrev Noralf Trønnes: >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >>> --- >>> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- >>> 1 file changed, 170 insertions(+), 9 deletions(-) >>> >> >> I have realised that this will change the DRM driver name from mi0283qt >> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] >> support. I haven't tried it, but this is the first thing that gives this >> driver any real advantage over its fbdev counterpart in >> drivers/staging/fbtft, so I don't want to loose that. >> So even if MIPI DBI panel support is implemented in some form, I will >> wait with converting mi0283qt until someone has updated the kmsro driver. > > Why does it change? You should be able to stuff whatever you feel like > into the drm driver name, this doesn't have to match either your > platform/spi/whatever driver name nor the module option. Strictly it doesn't have to change, I could pass a mi0283qt specific struct drm_driver to drm_mipi_dbi_panel_register() and keep the DRM driver name. Noralf. > -Daniel > > >> Noralf. >> >> [1] >> https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/winsys/kmsro/drm >> https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/targets/dri/target.c > > >
On 2019/07/30, Daniel Vetter wrote: > On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote: > > > > > > > > Den 29.07.2019 21.55, skrev Noralf Trønnes: > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > --- > > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- > > > 1 file changed, 170 insertions(+), 9 deletions(-) > > > > > > > I have realised that this will change the DRM driver name from mi0283qt > > to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] > > support. I haven't tried it, but this is the first thing that gives this > > driver any real advantage over its fbdev counterpart in > > drivers/staging/fbtft, so I don't want to loose that. > > So even if MIPI DBI panel support is implemented in some form, I will > > wait with converting mi0283qt until someone has updated the kmsro driver. > > Why does it change? You should be able to stuff whatever you feel like > into the drm driver name, this doesn't have to match either your > platform/spi/whatever driver name nor the module option. Last time i've looked DRM drivers using the mipi dsi helpers do _not_ have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi dbi should not have "drm_mipi_dbi". That said, we should probably highlight even more that the driver name is an ABI. -Emil
Den 30.07.2019 19.12, skrev Emil Velikov: > On 2019/07/30, Daniel Vetter wrote: >> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote: >>> >>> >>> >>> Den 29.07.2019 21.55, skrev Noralf Trønnes: >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >>>> --- >>>> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- >>>> 1 file changed, 170 insertions(+), 9 deletions(-) >>>> >>> >>> I have realised that this will change the DRM driver name from mi0283qt >>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] >>> support. I haven't tried it, but this is the first thing that gives this >>> driver any real advantage over its fbdev counterpart in >>> drivers/staging/fbtft, so I don't want to loose that. >>> So even if MIPI DBI panel support is implemented in some form, I will >>> wait with converting mi0283qt until someone has updated the kmsro driver. >> >> Why does it change? You should be able to stuff whatever you feel like >> into the drm driver name, this doesn't have to match either your >> platform/spi/whatever driver name nor the module option. > > Last time i've looked DRM drivers using the mipi dsi helpers do _not_ > have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi > dbi should not have "drm_mipi_dbi". > What purpose does the DRM driver name serve for userspace? Why can't it be called drm_mipi_dbi? Because multiple panel drivers will use the same name? You're statement implies that there are some rules regarding DRM driver naming. > That said, we should probably highlight even more that the driver name > is an ABI. > This I didn't know. Noralf.
On Tue, Jul 30, 2019 at 07:33:28PM +0200, Noralf Trønnes wrote: > > > Den 30.07.2019 19.12, skrev Emil Velikov: > > On 2019/07/30, Daniel Vetter wrote: > >> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote: > >>> > >>> > >>> > >>> Den 29.07.2019 21.55, skrev Noralf Trønnes: > >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>>> --- > >>>> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- > >>>> 1 file changed, 170 insertions(+), 9 deletions(-) > >>>> > >>> > >>> I have realised that this will change the DRM driver name from mi0283qt > >>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] > >>> support. I haven't tried it, but this is the first thing that gives this > >>> driver any real advantage over its fbdev counterpart in > >>> drivers/staging/fbtft, so I don't want to loose that. > >>> So even if MIPI DBI panel support is implemented in some form, I will > >>> wait with converting mi0283qt until someone has updated the kmsro driver. > >> > >> Why does it change? You should be able to stuff whatever you feel like > >> into the drm driver name, this doesn't have to match either your > >> platform/spi/whatever driver name nor the module option. > > > > Last time i've looked DRM drivers using the mipi dsi helpers do _not_ > > have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi > > dbi should not have "drm_mipi_dbi". > > > > What purpose does the DRM driver name serve for userspace? > Why can't it be called drm_mipi_dbi? Because multiple panel drivers will > use the same name? You're statement implies that there are some rules > regarding DRM driver naming. Worst case kmalloc a drm_driver at runtime and set the driver name to match the panel name? Imo that makes sense for these panel-as-full-drm_driver drivers ... > > That said, we should probably highlight even more that the driver name > > is an ABI. > > > > This I didn't know. kmsro and mesa in general uses it to figure out which userspace driver needs to be loaded for which kernel driver. That makes it uapi, and yeah I guess we should document it. I think aside from kmsro it's not relevant for display-only drivers, but for anything that does rendering and has custom ioctls it very much is the only real way to figure out what kind of driver you have. -Daniel
On Tue, 30 Jul 2019 at 18:33, Noralf Trønnes <noralf@tronnes.org> wrote: > Den 30.07.2019 19.12, skrev Emil Velikov: > > On 2019/07/30, Daniel Vetter wrote: > >> On Tue, Jul 30, 2019 at 4:30 PM Noralf Trønnes <noralf@tronnes.org> wrote: > >>> > >>> > >>> > >>> Den 29.07.2019 21.55, skrev Noralf Trønnes: > >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>>> --- > >>>> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- > >>>> 1 file changed, 170 insertions(+), 9 deletions(-) > >>>> > >>> > >>> I have realised that this will change the DRM driver name from mi0283qt > >>> to drm_mipi_dbi. This means that this panel will loose its MESA kmsro[1] > >>> support. I haven't tried it, but this is the first thing that gives this > >>> driver any real advantage over its fbdev counterpart in > >>> drivers/staging/fbtft, so I don't want to loose that. > >>> So even if MIPI DBI panel support is implemented in some form, I will > >>> wait with converting mi0283qt until someone has updated the kmsro driver. > >> > >> Why does it change? You should be able to stuff whatever you feel like > >> into the drm driver name, this doesn't have to match either your > >> platform/spi/whatever driver name nor the module option. > > > > Last time i've looked DRM drivers using the mipi dsi helpers do _not_ > > have "drm_mipi_dsi" as their driver name. Hence drivers using the mipi > > dbi should not have "drm_mipi_dbi". > > > > What purpose does the DRM driver name serve for userspace? > Why can't it be called drm_mipi_dbi? Because multiple panel drivers will > use the same name? You're statement implies that there are some rules > regarding DRM driver naming. > As you point out in the kmsro case - we use those to "pair" KMS with GPU device. We could use another, more robust solution, yet ETIME so this will do for now. > > That said, we should probably highlight even more that the driver name > > is an ABI. > > > > This I didn't know. > One simple example that has existed for years is amdgpu vs radeon distinction. In Mesa the radeonsi driver could use either driver. Although interacting with each one happens via a different set of ioctls. Additionally, a different set of features (and workarounds) is used depending on the version exposed by the driver. All these fields (and a bit more) are exposed to userspace via the drm_version() function/ioctl. HTH Emil
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index a755f3e60111..dd333a642159 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -21,10 +21,13 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/spi/spi.h> #include <video/mipi_display.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> #include <drm/drm_modes.h> #include <drm/drm_panel.h> #include <drm/drm_print.h> @@ -32,10 +35,25 @@ /* ILI9341 extended register set (Vendor Command Set) */ #define ILI9341_IFMODE 0xB0 // clock polarity +#define ILI9341_FRMCTR1 0xb1 +#define ILI9341_DISCTRL 0xb6 +#define ILI9341_ETMOD 0xb7 +#define ILI9341_PWCTRL1 0xc0 +#define ILI9341_PWCTRL2 0xc1 +#define ILI9341_VMCTRL1 0xc5 +#define ILI9341_VMCTRL2 0xc7 +#define ILI9341_PWCTRLA 0xcb +#define ILI9341_PWCTRLB 0xcf #define ILI9341_IFCTL 0xF6 // interface conrol #define ILI9341_PGAMCTRL 0xE0 // positive gamma control #define ILI9341_NGAMCTRL 0xE1 // negative gamma control +#define ILI9341_DTCTRLA 0xe8 +#define ILI9341_DTCTRLB 0xea +#define ILI9341_PWRSEQ 0xed +#define ILI9341_EN3GAM 0xf2 +#define ILI9341_PUMPCTRL 0xf7 +#define ILI9341_MADCTL_BGR BIT(3) #define ILI9341_MADCTL_MV BIT(5) #define ILI9341_MADCTL_MX BIT(6) #define ILI9341_MADCTL_MY BIT(7) @@ -44,15 +62,18 @@ * struct ili9341_config - the display specific configuration * @funcs: Panel functions * @mode: Display mode + * @command_mode_only: Panel only supports command mode */ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode; + bool command_mode_only; }; struct ili9341 { struct drm_panel panel; - struct mipi_dbi dbi; + struct mipi_dbi_dev dbidev; + bool command_mode; struct backlight_device *backlight; const struct ili9341_config *conf; }; @@ -64,7 +85,7 @@ static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) { - struct mipi_dbi *dbi = &ili->dbi; + struct mipi_dbi *dbi = &ili->dbidev.dbi; mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); mipi_dbi_command(dbi, MIPI_DCS_ENTER_SLEEP_MODE); @@ -74,7 +95,7 @@ static int ili9341_deinit(struct drm_panel *panel, struct ili9341 *ili) static int dt024ctft_init(struct drm_panel *panel, struct ili9341 *ili) { - struct mipi_dbi *dbi = &ili->dbi; + struct mipi_dbi *dbi = &ili->dbidev.dbi; /* HW / SW Reset display and wait */ mipi_dbi_hw_reset(dbi); @@ -164,6 +185,7 @@ static const struct drm_display_mode prgb_240x320_mode = { .height_mm = 49, }; +/* This is not used in command mode */ static int ili9341_get_modes(struct drm_panel *panel) { struct drm_connector *connector = panel->connector; @@ -201,19 +223,125 @@ static const struct ili9341_config dt024ctft_data = { .mode = &prgb_240x320_mode, }; +static int mi0283qt_prepare(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + struct mipi_dbi *dbi = &ili->dbidev.dbi; + u8 addr_mode; + int ret; + + DRM_DEBUG_KMS("\n"); + + ret = mipi_dbi_poweron_conditional_reset(&ili->dbidev); + if (ret < 0) + return ret; + if (ret == 1) + goto out_enable; + mipi_dbi_hw_reset(dbi); + + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); + + mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30); + mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81); + mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79); + mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02); + mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20); + mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00); + + /* Power Control */ + mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x26); + mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11); + /* VCOM */ + mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e); + mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe); + + /* Memory Access Control */ + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT); + + /* Frame Rate */ + mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b); + + /* Gamma */ + mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08); + mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01); + mipi_dbi_command(dbi, ILI9341_PGAMCTRL, + 0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87, + 0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00); + mipi_dbi_command(dbi, ILI9341_NGAMCTRL, + 0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78, + 0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f); + + /* DDRAM */ + mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07); + + /* Display */ + mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00); + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); + msleep(100); + + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON); + msleep(100); + +out_enable: + /* The PiTFT (ili9340) has a hardware reset circuit that + * resets only on power-on and not on each reboot through + * a gpio like the rpi-display does. + * As a result, we need to always apply the rotation value + * regardless of the display "on/off" state. + */ + switch (ili->dbidev.rotation) { + default: + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY | + ILI9341_MADCTL_MX; + break; + case 90: + addr_mode = ILI9341_MADCTL_MY; + break; + case 180: + addr_mode = ILI9341_MADCTL_MV; + break; + case 270: + addr_mode = ILI9341_MADCTL_MX; + break; + } + addr_mode |= ILI9341_MADCTL_BGR; + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); + + return 0; +} + +static const struct drm_panel_funcs mi0283qt_drm_funcs = { + .disable = ili9341_disable, + .unprepare = ili9341_unprepare, + .prepare = mi0283qt_prepare, + .enable = ili9341_enable, + .get_modes = ili9341_get_modes, +}; + +static const struct drm_display_mode mi0283qt_mode = { + DRM_SIMPLE_MODE(320, 240, 58, 43), +}; + +static const struct ili9341_config mi0283qt_data = { + .funcs = &mi0283qt_drm_funcs, + .mode = &mi0283qt_mode, + .command_mode_only = true, +}; + static int ili9341_probe(struct spi_device *spi) { struct device *dev = &spi->dev; struct ili9341 *ili; struct mipi_dbi *dbi; struct gpio_desc *dc_gpio; + struct device_node *port; int ret; - ili = devm_kzalloc(dev, sizeof(*ili), GFP_KERNEL); + ili = kzalloc(sizeof(*ili), GFP_KERNEL); if (!ili) return -ENOMEM; - dbi = &ili->dbi; + dbi = &ili->dbidev.dbi; spi_set_drvdata(spi, ili); @@ -255,23 +383,55 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs; - return drm_panel_add(&ili->panel); + port = of_get_child_by_name(dev->of_node, "port"); + if (port) + of_node_put(port); + else + ili->command_mode = true; + + if (ili->conf->command_mode_only) + ili->command_mode = true; + + if (ili->command_mode) + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, ili->conf->mode); + else + ret = drm_panel_add(&ili->panel); + + return ret; } static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi); - drm_panel_remove(&ili->panel); + if (ili->command_mode) { + struct drm_device *drm = &ili->dbidev.drm; - ili9341_disable(&ili->panel); - ili9341_unprepare(&ili->panel); + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + } else { + drm_panel_remove(&ili->panel); + + ili9341_disable(&ili->panel); + ili9341_unprepare(&ili->panel); + kfree(ili); + } return 0; } +static void ili9341_shutdown(struct spi_device *spi) +{ + struct ili9341 *ili = spi_get_drvdata(spi); + + if (ili->command_mode) + drm_atomic_helper_shutdown(&ili->dbidev.drm); +} + static const struct of_device_id ili9341_of_match[] = { { .compatible = "displaytech,dt024ctft", .data = &dt024ctft_data }, +/* { .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, */ + { .compatible = "mi,mi0283qt", .data = &mi0283qt_data }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, ili9341_of_match); @@ -279,6 +439,7 @@ MODULE_DEVICE_TABLE(of, ili9341_of_match); static struct spi_driver ili9341_driver = { .probe = ili9341_probe, .remove = ili9341_remove, + .shutdown = ili9341_shutdown, .driver = { .name = "panel-ilitek-ili9341", .of_match_table = ili9341_of_match,
Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 179 ++++++++++++++++++- 1 file changed, 170 insertions(+), 9 deletions(-)