Re: 答复: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

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

 



Hi Leon,

Le dim. 30 août 2020 à 16:36, 何小龙 (Leon He) <Leon.He@xxxxxxxxxx> a écrit :
 +struct ili9341 {
 +       struct drm_panel panel;
 +       struct mipi_dsi_device *dsi;
 +       const struct ili9341_pdata *pdata;
 +
 +       struct gpio_desc        *reset_gpiod;
 +       u32 rotation;
 +};
 +

Hi Paul, you put the mipi_dsi_device inside the struct. I think it maybe not a good idea. That means the panel has a MIPI-DSI interface but it doesn't
have actually.

 +static int ili9341_probe(struct mipi_dsi_device *dsi)
 +{
 +       struct device *dev = &dsi->dev;
 +       struct ili9341 *priv;
 +       int ret;
 +
 +       /* See comment for mipi_dbi_spi_init() */
 +       if (!dev->coherent_dma_mask) {
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
 +               if (ret) {
+ dev_warn(dev, "Failed to set dma mask %d\n", ret);
 +                       return ret;
 +               }
 +       }
 +
 +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 +       if (!priv)
 +               return -ENOMEM;
 +
 +       mipi_dsi_set_drvdata(dsi, priv);
 +       priv->dsi = dsi;
 +
 +       device_property_read_u32(dev, "rotation", &priv->rotation);
 +
 +       priv->pdata = device_get_match_data(dev);
 +       if (!priv->pdata)
 +               return -EINVAL;
 +
 +       drm_panel_init(&priv->panel, dev, &ili9341_funcs,
 +                      DRM_MODE_CONNECTOR_DPI);
 +
+ priv->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 +       if (IS_ERR(priv->reset_gpiod)) {
 +               dev_err(dev, "Couldn't get our reset GPIO\n");
 +               return PTR_ERR(priv->reset_gpiod);
 +       }
 +
 +       ret = drm_panel_of_backlight(&priv->panel);
 +       if (ret < 0) {
 +               if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get backlight handle\n");
 +               return ret;
 +       }
 +
 +       drm_panel_add(&priv->panel);
 +
 +       dsi->bus_type = priv->pdata->bus_type;
 +       dsi->lanes = priv->pdata->lanes;
 +       dsi->format = MIPI_DSI_FMT_RGB565;
 +
 +       ret = mipi_dsi_attach(dsi);
 +       if (ret) {
 +               dev_err(dev, "Failed to attach DSI panel\n");
 +               goto err_panel_remove;
 +       }
 +
 +       ret = mipi_dsi_maybe_register_tiny_driver(dsi);
 +       if (ret) {
 +               dev_err(dev, "Failed to init TinyDRM driver\n");
 +               goto err_mipi_dsi_detach;
 +       }
 +
 +       return 0;
 +
 +err_mipi_dsi_detach:
 +       mipi_dsi_detach(dsi);
 +err_panel_remove:
 +       drm_panel_remove(&priv->panel);
 +       return ret;
 +}
 +
 +static int ili9341_remove(struct mipi_dsi_device *dsi)
 +{
 +       struct ili9341 *priv = mipi_dsi_get_drvdata(dsi);
 +
 +       mipi_dsi_detach(dsi);
 +       drm_panel_remove(&priv->panel);
 +
 +       drm_panel_disable(&priv->panel);
 +       drm_panel_unprepare(&priv->panel);
 +
 +       return 0;
 +}
 +
 +static const struct ili9341_pdata yx240qv29_pdata = {
 +       .mode = { DRM_SIMPLE_MODE(240, 320, 37, 49) },
 +       .width_mm = 0, // TODO
 +       .height_mm = 0, // TODO
 +       .bus_type = MIPI_DCS_BUS_TYPE_DBI_SPI_C3,
 +       .lanes = 1,
 +};
 +
 +static const struct of_device_id ili9341_of_match[] = {
+ { .compatible = "adafruit,yx240qv29", .data = &yx240qv29_pdata },
 +       { }
 +};
 +MODULE_DEVICE_TABLE(of, ili9341_of_match);
 +
 +static struct mipi_dsi_driver ili9341_dsi_driver = {
 +       .probe          = ili9341_probe,
 +       .remove         = ili9341_remove,
 +       .driver = {
 +               .name           = "ili9341-dsi",
 +               .of_match_table = ili9341_of_match,
 +       },
 +};
 +module_mipi_dsi_driver(ili9341_dsi_driver);

Again, you treat this driver as a mipi dsi driver but for a MIPI-DBI (I8080/SPI)
panel device. That will make developers confused.

Is it possible to just add a mipi_dbi_driver for I8080/SPI interface panel?
Thanks!

Please read the cover letter, it explains why it's done this way. The whole point of this patchset is to merge DSI and DBI frameworks in a way that can be maintained.

Cheers,
-Paul






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux