Re: [RFC 4/4] drm/panel/ili9341: Support mi0283qt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxx> napsal:
>>
>> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
>> ---
>>  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
>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux