Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels
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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels
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. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels
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
>> ---
>> 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
>> static int ili9341_remove(struct spi_device *spi)
>> {
>> struct ili9341 *ili = spi_get_drvdata(spi);
>>
>> -drm_dev_unplug(>dbidev.drm);
>> -drm_atomic_helper_shutdown(>dbidev.drm);
>> +if (ili->use_dpi) {
>> +drm_panel_remove(>panel);
>> +drm_panel_disable(>panel);
>> +drm_panel_unprepare(>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(>dbidev.drm);
>> +drm_atomic_helper_shutdown(>dbidev.drm);
>> +}
>>
>> return 0;
>> }
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels
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. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels
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
> ---
> 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
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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 = _funcs,
> .mode = _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 = _drm_funcs,
> .mode = _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 = >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 == _data)
> - driver = _drm_driver;
> - else
> - driver = _drm_driver;
>
> - return drm_mipi_dbi_panel_register(>panel, >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(>panel);
> + } else {
> + if (ili->conf == _data)
> + driver = _drm_driver;
> + else
> + driver = _drm_driver;
> +
> + ret = drm_mipi_dbi_panel_register(>panel, >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(>dbidev.drm);
> - drm_atomic_helper_shutdown(>dbidev.drm);
> + if (ili->use_dpi) {
> + drm_panel_remove(>panel);
> + drm_panel_disable(>panel);
> + drm_panel_unprepare(>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(>dbidev.drm);
> + drm_atomic_helper_shutdown(>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(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_atomic_helper_shutdown(>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(>dbidev.drm);
> + if (!ili->use_dpi)
> + return drm_mode_config_helper_suspend(>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(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_mode_config_helper_resume(>dbidev.drm);
>
> return 0;
> }
> --
> 2.20.1
___
dri-devel mailing list
Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels
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
> ---
> 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
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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 = _funcs,
> .mode = _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 = _drm_funcs,
> .mode = _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 = >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 == _data)
> - driver = _drm_driver;
> - else
> - driver = _drm_driver;
>
> - return drm_mipi_dbi_panel_register(>panel, >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(>panel);
> + } else {
> + if (ili->conf == _data)
> + driver = _drm_driver;
> + else
> + driver = _drm_driver;
> +
> + ret = drm_mipi_dbi_panel_register(>panel, >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(>dbidev.drm);
> - drm_atomic_helper_shutdown(>dbidev.drm);
> + if (ili->use_dpi) {
> + drm_panel_remove(>panel);
> + drm_panel_disable(>panel);
> + drm_panel_unprepare(>panel);
> + kfree(ili);
> + } else {
> + drm_dev_unplug(>dbidev.drm);
> + drm_atomic_helper_shutdown(>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(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_atomic_helper_shutdown(>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(>dbidev.drm);
> + if (!ili->use_dpi)
> + return drm_mode_config_helper_suspend(>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(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_mode_config_helper_resume(>dbidev.drm);
>
> return 0;
> }
> --
> 2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
[PATCH 4/4] drm/panel/ili9341: Support DPI panels
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
---
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
#include
#include
+#include
#include
#include
#include
@@ -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 = _funcs,
.mode = _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 = _drm_funcs,
.mode = _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 = >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 == _data)
- driver = _drm_driver;
- else
- driver = _drm_driver;
- return drm_mipi_dbi_panel_register(>panel, >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(>panel);
+ } else {
+ if (ili->conf == _data)
+ driver = _drm_driver;
+ else
+ driver = _drm_driver;
+
+ ret = drm_mipi_dbi_panel_register(>panel, >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(>dbidev.drm);
- drm_atomic_helper_shutdown(>dbidev.drm);
+ if (ili->use_dpi) {
+ drm_panel_remove(>panel);
+ drm_panel_disable(>panel);
+ drm_panel_unprepare(>panel);
+ kfree(ili);
+ } else {
+ drm_dev_unplug(>dbidev.drm);
+ drm_atomic_helper_shutdown(>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(>dbidev.drm);
+ if (!ili->use_dpi)
+ drm_atomic_helper_shutdown(>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(>dbidev.drm);
+ if (!ili->use_dpi)
+ return drm_mode_config_helper_suspend(>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(>dbidev.drm);
+ if (!ili->use_dpi)
+ drm_mode_config_helper_resume(>dbidev.drm);
return 0;
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
