| Message ID | 20190801135249.28803-5-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mipi-dbi: Support panel drivers | expand |
On 8/1/19 8:52 AM, Noralf Trønnes wrote: > Add support for panels that use the DPI interface. > ILI9341 has onboard RAM so the assumption made here is that all such > panels support pixel upload over DBI. > > The presence/absense of the Device Tree 'port' node decides which > interface is used for pixel transfer. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- > 1 file changed, 45 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > index f6082fa2a389..7cbfd739c7fd 100644 > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > @@ -11,6 +11,7 @@ > #include <linux/gpio/consumer.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of_graph.h> > #include <linux/pm.h> > #include <linux/property.h> > #include <linux/regulator/consumer.h> > @@ -53,11 +54,13 @@ > struct ili9341_config { > const struct drm_panel_funcs *funcs; > const struct drm_display_mode *mode; > + bool no_dpi; Can we invert this to use positive logic? i.e. use_dpi instead of no_dpi. > }; > > struct ili9341 { > struct mipi_dbi_dev dbidev; /* This must be the first entry */ > struct drm_panel panel; > + bool use_dpi; > struct regulator *regulator; > struct backlight_device *backlight; > const struct ili9341_config *conf; > @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { > static const struct ili9341_config yx240qv29_data = { > .funcs = &yx240qv29_funcs, > .mode = &yx240qv29_mode, > + .no_dpi = true, > }; > > static int mi0283qt_prepare(struct drm_panel *panel) > @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { > static const struct ili9341_config mi0283qt_data = { > .funcs = &mi0283qt_drm_funcs, > .mode = &mi0283qt_mode, > + .no_dpi = true, > }; > > /* Legacy, DRM driver name is ABI */ > @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) > const struct spi_device_id *spi_id; > struct device *dev = &spi->dev; > struct drm_driver *driver; > + struct device_node *port; > struct mipi_dbi *dbi; > struct gpio_desc *dc; > struct ili9341 *ili; > @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) > ili->panel.dev = dev; > ili->panel.funcs = ili->conf->funcs; > > - if (ili->conf == &mi0283qt_data) > - driver = &mi0283qt_drm_driver; > - else > - driver = &ili9341_drm_driver; > > - return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, > - ili->conf->mode, rotation); > + port = of_get_child_by_name(dev->of_node, "port"); > + if (port) { > + of_node_put(port); > + ili->use_dpi = true; > + } > + > + if (ili->conf->no_dpi) > + ili->use_dpi = false; then this can just be ili->use_dpi = ili->conf->use_dpi. > + > + if (ili->use_dpi) { > + ret = drm_panel_add(&ili->panel); > + } else { > + if (ili->conf == &mi0283qt_data) > + driver = &mi0283qt_drm_driver; > + else > + driver = &ili9341_drm_driver; > + > + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, > + ili->conf->mode, rotation); > + } > + > + return ret; > } > > static int ili9341_remove(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_dev_unplug(&ili->dbidev.drm); > - drm_atomic_helper_shutdown(&ili->dbidev.drm); > + if (ili->use_dpi) { > + drm_panel_remove(&ili->panel); > + drm_panel_disable(&ili->panel); > + drm_panel_unprepare(&ili->panel); > + kfree(ili); > + } else { > + drm_dev_unplug(&ili->dbidev.drm); > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > + } > > return 0; > } > @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_atomic_helper_shutdown(&ili->dbidev.drm); > + if (!ili->use_dpi) > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > } > > static int __maybe_unused ili9341_pm_suspend(struct device *dev) > { > struct ili9341 *ili = dev_get_drvdata(dev); > > - return drm_mode_config_helper_suspend(&ili->dbidev.drm); > + if (!ili->use_dpi) > + return drm_mode_config_helper_suspend(&ili->dbidev.drm); > + > + return 0; > } > > static int __maybe_unused ili9341_pm_resume(struct device *dev) > { > struct ili9341 *ili = dev_get_drvdata(dev); > > - drm_mode_config_helper_resume(&ili->dbidev.drm); > + if (!ili->use_dpi) > + drm_mode_config_helper_resume(&ili->dbidev.drm); > > return 0; > } >
Den 01.08.2019 21.10, skrev David Lechner: > On 8/1/19 8:52 AM, Noralf Trønnes wrote: >> Add support for panels that use the DPI interface. >> ILI9341 has onboard RAM so the assumption made here is that all such >> panels support pixel upload over DBI. >> >> The presence/absense of the Device Tree 'port' node decides which >> interface is used for pixel transfer. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- >> 1 file changed, 45 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> index f6082fa2a389..7cbfd739c7fd 100644 >> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> @@ -11,6 +11,7 @@ >> #include <linux/gpio/consumer.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> +#include <linux/of_graph.h> >> #include <linux/pm.h> >> #include <linux/property.h> >> #include <linux/regulator/consumer.h> >> @@ -53,11 +54,13 @@ >> struct ili9341_config { >> const struct drm_panel_funcs *funcs; >> const struct drm_display_mode *mode; >> + bool no_dpi; > > Can we invert this to use positive logic? i.e. use_dpi instead of > no_dpi. > This is a capability flag. I could call it has_dpi and add a comment about what it does. Noralf. >> }; >> struct ili9341 { >> struct mipi_dbi_dev dbidev; /* This must be the first entry */ >> struct drm_panel panel; >> + bool use_dpi; >> struct regulator *regulator; >> struct backlight_device *backlight; >> const struct ili9341_config *conf; >> @@ -174,6 +177,7 @@ static const struct drm_display_mode >> yx240qv29_mode = { >> static const struct ili9341_config yx240qv29_data = { >> .funcs = &yx240qv29_funcs, >> .mode = &yx240qv29_mode, >> + .no_dpi = true, >> }; >> static int mi0283qt_prepare(struct drm_panel *panel) >> @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode >> = { >> static const struct ili9341_config mi0283qt_data = { >> .funcs = &mi0283qt_drm_funcs, >> .mode = &mi0283qt_mode, >> + .no_dpi = true, >> }; >> /* Legacy, DRM driver name is ABI */ >> @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) >> const struct spi_device_id *spi_id; >> struct device *dev = &spi->dev; >> struct drm_driver *driver; >> + struct device_node *port; >> struct mipi_dbi *dbi; >> struct gpio_desc *dc; >> struct ili9341 *ili; >> @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) >> ili->panel.dev = dev; >> ili->panel.funcs = ili->conf->funcs; >> - if (ili->conf == &mi0283qt_data) >> - driver = &mi0283qt_drm_driver; >> - else >> - driver = &ili9341_drm_driver; >> - return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, >> driver, >> - ili->conf->mode, rotation); >> + port = of_get_child_by_name(dev->of_node, "port"); >> + if (port) { >> + of_node_put(port); >> + ili->use_dpi = true; >> + } >> + >> + if (ili->conf->no_dpi) >> + ili->use_dpi = false; > > then this can just be ili->use_dpi = ili->conf->use_dpi. > >> + >> + if (ili->use_dpi) { >> + ret = drm_panel_add(&ili->panel); >> + } else { >> + if (ili->conf == &mi0283qt_data) >> + driver = &mi0283qt_drm_driver; >> + else >> + driver = &ili9341_drm_driver; >> + >> + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, >> driver, >> + ili->conf->mode, rotation); >> + } >> + >> + return ret; >> } >> static int ili9341_remove(struct spi_device *spi) >> { >> struct ili9341 *ili = spi_get_drvdata(spi); >> - drm_dev_unplug(&ili->dbidev.drm); >> - drm_atomic_helper_shutdown(&ili->dbidev.drm); >> + if (ili->use_dpi) { >> + drm_panel_remove(&ili->panel); >> + drm_panel_disable(&ili->panel); >> + drm_panel_unprepare(&ili->panel); >> + kfree(ili); >> + } else { >> + drm_dev_unplug(&ili->dbidev.drm); >> + drm_atomic_helper_shutdown(&ili->dbidev.drm); >> + } >> return 0; >> } >> @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device >> *spi) >> { >> struct ili9341 *ili = spi_get_drvdata(spi); >> - drm_atomic_helper_shutdown(&ili->dbidev.drm); >> + if (!ili->use_dpi) >> + drm_atomic_helper_shutdown(&ili->dbidev.drm); >> } >> static int __maybe_unused ili9341_pm_suspend(struct device *dev) >> { >> struct ili9341 *ili = dev_get_drvdata(dev); >> - return drm_mode_config_helper_suspend(&ili->dbidev.drm); >> + if (!ili->use_dpi) >> + return drm_mode_config_helper_suspend(&ili->dbidev.drm); >> + >> + return 0; >> } >> static int __maybe_unused ili9341_pm_resume(struct device *dev) >> { >> struct ili9341 *ili = dev_get_drvdata(dev); >> - drm_mode_config_helper_resume(&ili->dbidev.drm); >> + if (!ili->use_dpi) >> + drm_mode_config_helper_resume(&ili->dbidev.drm); >> return 0; >> } >> >
Hi Noralf. On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote: > Add support for panels that use the DPI interface. > ILI9341 has onboard RAM so the assumption made here is that all such > panels support pixel upload over DBI. > > The presence/absense of the Device Tree 'port' node decides which > interface is used for pixel transfer. Have you consiered if the compatible could be used to determine the use of a panel? Then it is more explicit how the HW is described in DT. Sam > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- > 1 file changed, 45 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > index f6082fa2a389..7cbfd739c7fd 100644 > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > @@ -11,6 +11,7 @@ > #include <linux/gpio/consumer.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of_graph.h> > #include <linux/pm.h> > #include <linux/property.h> > #include <linux/regulator/consumer.h> > @@ -53,11 +54,13 @@ > struct ili9341_config { > const struct drm_panel_funcs *funcs; > const struct drm_display_mode *mode; > + bool no_dpi; > }; > > struct ili9341 { > struct mipi_dbi_dev dbidev; /* This must be the first entry */ > struct drm_panel panel; > + bool use_dpi; > struct regulator *regulator; > struct backlight_device *backlight; > const struct ili9341_config *conf; > @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { > static const struct ili9341_config yx240qv29_data = { > .funcs = &yx240qv29_funcs, > .mode = &yx240qv29_mode, > + .no_dpi = true, > }; > > static int mi0283qt_prepare(struct drm_panel *panel) > @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { > static const struct ili9341_config mi0283qt_data = { > .funcs = &mi0283qt_drm_funcs, > .mode = &mi0283qt_mode, > + .no_dpi = true, > }; > > /* Legacy, DRM driver name is ABI */ > @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) > const struct spi_device_id *spi_id; > struct device *dev = &spi->dev; > struct drm_driver *driver; > + struct device_node *port; > struct mipi_dbi *dbi; > struct gpio_desc *dc; > struct ili9341 *ili; > @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) > ili->panel.dev = dev; > ili->panel.funcs = ili->conf->funcs; > > - if (ili->conf == &mi0283qt_data) > - driver = &mi0283qt_drm_driver; > - else > - driver = &ili9341_drm_driver; > > - return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, > - ili->conf->mode, rotation); > + port = of_get_child_by_name(dev->of_node, "port"); > + if (port) { > + of_node_put(port); > + ili->use_dpi = true; > + } > + > + if (ili->conf->no_dpi) > + ili->use_dpi = false; > + > + if (ili->use_dpi) { > + ret = drm_panel_add(&ili->panel); > + } else { > + if (ili->conf == &mi0283qt_data) > + driver = &mi0283qt_drm_driver; > + else > + driver = &ili9341_drm_driver; > + > + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, > + ili->conf->mode, rotation); > + } > + > + return ret; > } > > static int ili9341_remove(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_dev_unplug(&ili->dbidev.drm); > - drm_atomic_helper_shutdown(&ili->dbidev.drm); > + if (ili->use_dpi) { > + drm_panel_remove(&ili->panel); > + drm_panel_disable(&ili->panel); > + drm_panel_unprepare(&ili->panel); > + kfree(ili); > + } else { > + drm_dev_unplug(&ili->dbidev.drm); > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > + } > > return 0; > } > @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_atomic_helper_shutdown(&ili->dbidev.drm); > + if (!ili->use_dpi) > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > } > > static int __maybe_unused ili9341_pm_suspend(struct device *dev) > { > struct ili9341 *ili = dev_get_drvdata(dev); > > - return drm_mode_config_helper_suspend(&ili->dbidev.drm); > + if (!ili->use_dpi) > + return drm_mode_config_helper_suspend(&ili->dbidev.drm); > + > + return 0; > } > > static int __maybe_unused ili9341_pm_resume(struct device *dev) > { > struct ili9341 *ili = dev_get_drvdata(dev); > > - drm_mode_config_helper_resume(&ili->dbidev.drm); > + if (!ili->use_dpi) > + drm_mode_config_helper_resume(&ili->dbidev.drm); > > return 0; > } > -- > 2.20.1
Hi Noralf. On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote: > Add support for panels that use the DPI interface. > ILI9341 has onboard RAM so the assumption made here is that all such > panels support pixel upload over DBI. > > The presence/absense of the Device Tree 'port' node decides which > interface is used for pixel transfer. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- > 1 file changed, 45 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > index f6082fa2a389..7cbfd739c7fd 100644 > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > @@ -11,6 +11,7 @@ > #include <linux/gpio/consumer.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of_graph.h> > #include <linux/pm.h> > #include <linux/property.h> > #include <linux/regulator/consumer.h> > @@ -53,11 +54,13 @@ > struct ili9341_config { > const struct drm_panel_funcs *funcs; > const struct drm_display_mode *mode; > + bool no_dpi; > }; > > struct ili9341 { > struct mipi_dbi_dev dbidev; /* This must be the first entry */ > struct drm_panel panel; > + bool use_dpi; > struct regulator *regulator; > struct backlight_device *backlight; > const struct ili9341_config *conf; > @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { > static const struct ili9341_config yx240qv29_data = { > .funcs = &yx240qv29_funcs, > .mode = &yx240qv29_mode, > + .no_dpi = true, > }; > > static int mi0283qt_prepare(struct drm_panel *panel) > @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { > static const struct ili9341_config mi0283qt_data = { > .funcs = &mi0283qt_drm_funcs, > .mode = &mi0283qt_mode, > + .no_dpi = true, > }; > > /* Legacy, DRM driver name is ABI */ > @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) > const struct spi_device_id *spi_id; > struct device *dev = &spi->dev; > struct drm_driver *driver; > + struct device_node *port; > struct mipi_dbi *dbi; > struct gpio_desc *dc; > struct ili9341 *ili; > @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) > ili->panel.dev = dev; > ili->panel.funcs = ili->conf->funcs; > > - if (ili->conf == &mi0283qt_data) > - driver = &mi0283qt_drm_driver; > - else > - driver = &ili9341_drm_driver; > > - return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, > - ili->conf->mode, rotation); > + port = of_get_child_by_name(dev->of_node, "port"); > + if (port) { > + of_node_put(port); > + ili->use_dpi = true; > + } > + > + if (ili->conf->no_dpi) > + ili->use_dpi = false; > + > + if (ili->use_dpi) { > + ret = drm_panel_add(&ili->panel); > + } else { > + if (ili->conf == &mi0283qt_data) > + driver = &mi0283qt_drm_driver; > + else > + driver = &ili9341_drm_driver; > + > + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, > + ili->conf->mode, rotation); > + } > + > + return ret; > } > > static int ili9341_remove(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_dev_unplug(&ili->dbidev.drm); > - drm_atomic_helper_shutdown(&ili->dbidev.drm); > + if (ili->use_dpi) { > + drm_panel_remove(&ili->panel); > + drm_panel_disable(&ili->panel); > + drm_panel_unprepare(&ili->panel); > + kfree(ili); At first I thought - order is wrong. But drm_panel_remove() prevents display drivers from using the driver. And this will not invalidate the other calls. Maybe add a short comment? Sam > + } else { > + drm_dev_unplug(&ili->dbidev.drm); > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > + } > > return 0; > } > @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) > { > struct ili9341 *ili = spi_get_drvdata(spi); > > - drm_atomic_helper_shutdown(&ili->dbidev.drm); > + if (!ili->use_dpi) > + drm_atomic_helper_shutdown(&ili->dbidev.drm); > } > > static int __maybe_unused ili9341_pm_suspend(struct device *dev) > { > struct ili9341 *ili = dev_get_drvdata(dev); > > - return drm_mode_config_helper_suspend(&ili->dbidev.drm); > + if (!ili->use_dpi) > + return drm_mode_config_helper_suspend(&ili->dbidev.drm); > + > + return 0; > } > > static int __maybe_unused ili9341_pm_resume(struct device *dev) > { > struct ili9341 *ili = dev_get_drvdata(dev); > > - drm_mode_config_helper_resume(&ili->dbidev.drm); > + if (!ili->use_dpi) > + drm_mode_config_helper_resume(&ili->dbidev.drm); > > return 0; > } > -- > 2.20.1
Den 11.08.2019 18.41, skrev Sam Ravnborg: > Hi Noralf. > > On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote: >> Add support for panels that use the DPI interface. >> ILI9341 has onboard RAM so the assumption made here is that all such >> panels support pixel upload over DBI. >> >> The presence/absense of the Device Tree 'port' node decides which >> interface is used for pixel transfer. > Have you consiered if the compatible could be used to determine the use > of a panel? > Then it is more explicit how the HW is described in DT. > Is that common to use the compatible to tell which interface to use? I don't know what best practice is here. Noralf.
Den 11.08.2019 19.02, skrev Sam Ravnborg: > Hi Noralf. > > On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote: >> Add support for panels that use the DPI interface. >> ILI9341 has onboard RAM so the assumption made here is that all such >> panels support pixel upload over DBI. >> >> The presence/absense of the Device Tree 'port' node decides which >> interface is used for pixel transfer. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- >> 1 file changed, 45 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c <snip> >> static int ili9341_remove(struct spi_device *spi) >> { >> struct ili9341 *ili = spi_get_drvdata(spi); >> >> - drm_dev_unplug(&ili->dbidev.drm); >> - drm_atomic_helper_shutdown(&ili->dbidev.drm); >> + if (ili->use_dpi) { >> + drm_panel_remove(&ili->panel); >> + drm_panel_disable(&ili->panel); >> + drm_panel_unprepare(&ili->panel); >> + kfree(ili); > At first I thought - order is wrong. > But drm_panel_remove() prevents display drivers from using the driver. > And this will not invalidate the other calls. > Maybe add a short comment? > I just copied this code from Josef's driver, didn't actually look that close at it. Isn't there a common pattern for this in the panel drivers? I would assume that everyone would have to do more or less the same on driver unbind. Noralf. > Sam > > >> + } else { >> + drm_dev_unplug(&ili->dbidev.drm); >> + drm_atomic_helper_shutdown(&ili->dbidev.drm); >> + } >> >> return 0; >> }
On Mon, Aug 12, 2019 at 02:13:54PM +0200, Noralf Trønnes wrote: > Den 11.08.2019 18.41, skrev Sam Ravnborg: > > On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote: > >> Add support for panels that use the DPI interface. > >> ILI9341 has onboard RAM so the assumption made here is that all such > >> panels support pixel upload over DBI. > >> > >> The presence/absense of the Device Tree 'port' node decides which > >> interface is used for pixel transfer. > > > > Have you consiered if the compatible could be used to determine the use > > of a panel? Then it is more explicit how the HW is described in DT. > > Is that common to use the compatible to tell which interface to use? > I don't know what best practice is here. Usually the compatible identifies the device, not the interface. Additional properties are preferred to describe the interface.
Hi Laurent/Noralf. On Mon, Aug 12, 2019 at 06:35:42PM +0300, Laurent Pinchart wrote: > On Mon, Aug 12, 2019 at 02:13:54PM +0200, Noralf Trønnes wrote: > > Den 11.08.2019 18.41, skrev Sam Ravnborg: > > > On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote: > > >> Add support for panels that use the DPI interface. > > >> ILI9341 has onboard RAM so the assumption made here is that all such > > >> panels support pixel upload over DBI. > > >> > > >> The presence/absense of the Device Tree 'port' node decides which > > >> interface is used for pixel transfer. > > > > > > Have you consiered if the compatible could be used to determine the use > > > of a panel? Then it is more explicit how the HW is described in DT. > > > > Is that common to use the compatible to tell which interface to use? > > I don't know what best practice is here. > > Usually the compatible identifies the device, not the interface. > Additional properties are preferred to describe the interface. Thanks Laurent. Then the ports idea as implmented by the patch seems to fly. Sam
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f6082fa2a389..7cbfd739c7fd 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -11,6 +11,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> @@ -53,11 +54,13 @@ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode; + bool no_dpi; }; struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel; + bool use_dpi; struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf; @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { static const struct ili9341_config yx240qv29_data = { .funcs = &yx240qv29_funcs, .mode = &yx240qv29_mode, + .no_dpi = true, }; static int mi0283qt_prepare(struct drm_panel *panel) @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { static const struct ili9341_config mi0283qt_data = { .funcs = &mi0283qt_drm_funcs, .mode = &mi0283qt_mode, + .no_dpi = true, }; /* Legacy, DRM driver name is ABI */ @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) const struct spi_device_id *spi_id; struct device *dev = &spi->dev; struct drm_driver *driver; + struct device_node *port; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili; @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs; - if (ili->conf == &mi0283qt_data) - driver = &mi0283qt_drm_driver; - else - driver = &ili9341_drm_driver; - return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, - ili->conf->mode, rotation); + port = of_get_child_by_name(dev->of_node, "port"); + if (port) { + of_node_put(port); + ili->use_dpi = true; + } + + if (ili->conf->no_dpi) + ili->use_dpi = false; + + if (ili->use_dpi) { + ret = drm_panel_add(&ili->panel); + } else { + if (ili->conf == &mi0283qt_data) + driver = &mi0283qt_drm_driver; + else + driver = &ili9341_drm_driver; + + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, + ili->conf->mode, rotation); + } + + return ret; } static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi); - drm_dev_unplug(&ili->dbidev.drm); - drm_atomic_helper_shutdown(&ili->dbidev.drm); + if (ili->use_dpi) { + drm_panel_remove(&ili->panel); + drm_panel_disable(&ili->panel); + drm_panel_unprepare(&ili->panel); + kfree(ili); + } else { + drm_dev_unplug(&ili->dbidev.drm); + drm_atomic_helper_shutdown(&ili->dbidev.drm); + } return 0; } @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi); - drm_atomic_helper_shutdown(&ili->dbidev.drm); + if (!ili->use_dpi) + drm_atomic_helper_shutdown(&ili->dbidev.drm); } static int __maybe_unused ili9341_pm_suspend(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev); - return drm_mode_config_helper_suspend(&ili->dbidev.drm); + if (!ili->use_dpi) + return drm_mode_config_helper_suspend(&ili->dbidev.drm); + + return 0; } static int __maybe_unused ili9341_pm_resume(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev); - drm_mode_config_helper_resume(&ili->dbidev.drm); + if (!ili->use_dpi) + drm_mode_config_helper_resume(&ili->dbidev.drm); return 0; }
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI. The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)