This is one of several different panels that are used on the Sony Xperia Z3 phone. It can operate in either command or video mode, although so far only video mode is implemented (since that is the mode that the downstream kernel version I happened to be looking at was using, and that is where I got the programming sequences for the panel).
Signed-off-by: Rob Clark robdclark@gmail.com --- Couple Notes: 1) programming sequences and basically everything I know about the panel came from downstream android kernel. I've started a wiki page to document how to translate from downstream kernel-msm way of doing things to upstream panel framework, which may be useful for others wanting to port downstream panel drivers for snapdragon devices:
https://github.com/freedreno/freedreno/wiki/DSI-Panel-Driver-Porting
2) The Sony phones at least (not sure if this is common) use one of several different panels, with runtime probing. Depending on the device they seem to either use a gpio (simple) or send some DSI commands to read back the panel-id. My rough thinking here about how to handle this is to implement a "panel-meta" driver (or maybe one each for the different probing methods), which would take a list of phandles to the actual candidate panels, and fwd the panel fxn calls to the chosen panel after probing.
.../bindings/panel/auo,novatek-1080p.txt | 25 ++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-auo-novatek-1080p.c | 470 +++++++++++++++++++++ 4 files changed, 505 insertions(+) create mode 100644 Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt create mode 100644 drivers/gpu/drm/panel/panel-auo-novatek-1080p.c
diff --git a/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt new file mode 100644 index 0000000..8a53093 --- /dev/null +++ b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt @@ -0,0 +1,25 @@ +AU Optronics Corporation 1080x1920 DSI panel + +This panel supports both video and command mode (although currently only video +mode is implemented in the driver. + +Required properties: +- compatible: should be "auo,novatek-1080p-vid" + +Optional properties: +- power-supply: phandle of the regulator that provides the supply voltage +- reset-gpio: phandle of gpio for reset line +- backlight: phandle of the backlight device attached to the panel + +Example: + + dsi@54300000 { + panel: panel@0 { + compatible = "auo,novatek-1080p-vid"; + reg = <0>; + + power-supply = <...>; + reset-gpio = <...>; + backlight = <...>; + }; + }; diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 6d64c7b..89f0e8c 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -43,4 +43,13 @@ config DRM_PANEL_SHARP_LQ101R1SX01 To compile this driver as a module, choose M here: the module will be called panel-sharp-lq101r1sx01.
+config DRM_PANEL_AUO_NOVATEK_1080P + tristate "AUO Novatek 1080p video mode panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for AUO 1080p DSI panel + as found in some Sony XPERIA Z3 devices + endmenu diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 4b2a043..cbcfedf 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o +obj-$(CONFIG_DRM_PANEL_AUO_NOVATEK_1080P) += panel-auo-novatek-1080p.o diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c new file mode 100644 index 0000000..0db70dd --- /dev/null +++ b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c @@ -0,0 +1,470 @@ +/* + * Copyright (C) 2015 Red Hat + * Author: Rob Clark robdclark@gmail.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see http://www.gnu.org/licenses/. + */ + +#include <linux/backlight.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> + +#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h> + +#include <video/mipi_display.h> + +struct auo_panel { + struct drm_panel base; + struct mipi_dsi_device *dsi; + + struct backlight_device *backlight; + struct regulator *supply; + struct gpio_desc *reset_gpio; + + bool prepared; + bool enabled; + + const struct drm_display_mode *mode; +}; + +static inline struct auo_panel *to_auo_panel(struct drm_panel *panel) +{ + return container_of(panel, struct auo_panel, base); +} + +static int auo_panel_init(struct auo_panel *auo) +{ + struct mipi_dsi_device *dsi = auo->dsi; + int ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0xe0 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xb5, (u8[]){ 0x86 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xb6, (u8[]){ 0x77 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xb8, (u8[]){ 0xad }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x20 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x24 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xc6, (u8[]){ 0x00 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xc5, (u8[]){ 0x32 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0x92, (u8[]){ 0x92 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x10 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, + (u8[]){ 0x03, 0x00 }, 2); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0x3b, (u8[]){ 0x03, 0x30, 0x06 }, 3); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_write(dsi, 0xbb, (u8[]){ 0x10 }, 1); + if (ret < 0) + return ret; + msleep(1); + + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); + if (ret < 0) + return ret; + msleep(30); + + return 0; +} + +static int auo_panel_on(struct auo_panel *auo) +{ + struct mipi_dsi_device *dsi = auo->dsi; + int ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_set_display_on(dsi); + if (ret < 0) + return ret; + + msleep(40); + + return 0; +} + +static int auo_panel_off(struct auo_panel *auo) +{ + struct mipi_dsi_device *dsi = auo->dsi; + int ret; + + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x10 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_set_display_off(dsi); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + if (ret < 0) + return ret; + + msleep(100); + + return 0; +} + +static int auo_panel_disable(struct drm_panel *panel) +{ + struct auo_panel *auo = to_auo_panel(panel); + + if (!auo->enabled) + return 0; + + DRM_DEBUG("disable\n"); + + if (auo->backlight) { + auo->backlight->props.power = FB_BLANK_POWERDOWN; + backlight_update_status(auo->backlight); + } + + auo->enabled = false; + + return 0; +} + +static int auo_panel_unprepare(struct drm_panel *panel) +{ + struct auo_panel *auo = to_auo_panel(panel); + int ret; + + if (!auo->prepared) + return 0; + + DRM_DEBUG("unprepare\n"); + + ret = auo_panel_off(auo); + if (ret) { + dev_err(panel->dev, "failed to set panel off: %d\n", ret); + return ret; + } + + regulator_disable(auo->supply); + if (auo->reset_gpio) + gpiod_set_value(auo->reset_gpio, 0); + + auo->prepared = false; + + return 0; +} + +static int auo_panel_prepare(struct drm_panel *panel) +{ + struct auo_panel *auo = to_auo_panel(panel); + int ret; + + if (auo->prepared) + return 0; + + DRM_DEBUG("prepare\n"); + + if (auo->reset_gpio) { + gpiod_set_value(auo->reset_gpio, 0); + msleep(5); + } + + ret = regulator_enable(auo->supply); + if (ret < 0) + return ret; + + msleep(20); + + if (auo->reset_gpio) { + gpiod_set_value(auo->reset_gpio, 1); + msleep(10); + } + + msleep(150); + + ret = auo_panel_init(auo); + if (ret) { + dev_err(panel->dev, "failed to init panel: %d\n", ret); + goto poweroff; + } + + ret = auo_panel_on(auo); + if (ret) { + dev_err(panel->dev, "failed to set panel on: %d\n", ret); + goto poweroff; + } + + auo->prepared = true; + + return 0; + +poweroff: + regulator_disable(auo->supply); + if (auo->reset_gpio) + gpiod_set_value(auo->reset_gpio, 0); + return ret; +} + +static int auo_panel_enable(struct drm_panel *panel) +{ + struct auo_panel *auo = to_auo_panel(panel); + + if (auo->enabled) + return 0; + + DRM_DEBUG("enable\n"); + + if (auo->backlight) { + auo->backlight->props.power = FB_BLANK_UNBLANK; + backlight_update_status(auo->backlight); + } + + auo->enabled = true; + + return 0; +} + +static const struct drm_display_mode default_mode = { + .clock = 149506, + .hdisplay = 1080, + .hsync_start = 1080 + 56, + .hsync_end = 1080 + 56 + 8, + .htotal = 1080 + 56 + 8 + 8, + .vdisplay = 1920, + .vsync_start = 1920 + 233, + .vsync_end = 1920 + 233 + 2, + .vtotal = 1920 + 233 + 2 + 8, + .vrefresh = 60, +}; + +static int auo_panel_get_modes(struct drm_panel *panel) +{ + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(panel->drm, &default_mode); + if (!mode) { + dev_err(panel->drm->dev, "failed to add mode %ux%ux@%u\n", + default_mode.hdisplay, default_mode.vdisplay, + default_mode.vrefresh); + return -ENOMEM; + } + + drm_mode_set_name(mode); + + drm_mode_probed_add(panel->connector, mode); + + panel->connector->display_info.width_mm = 64; + panel->connector->display_info.height_mm = 114; + + return 1; +} + +static const struct drm_panel_funcs auo_panel_funcs = { + .disable = auo_panel_disable, + .unprepare = auo_panel_unprepare, + .prepare = auo_panel_prepare, + .enable = auo_panel_enable, + .get_modes = auo_panel_get_modes, +}; + +static const struct of_device_id auo_of_match[] = { + { .compatible = "auo,novatek-1080p-vid", }, + { } +}; +MODULE_DEVICE_TABLE(of, auo_of_match); + +static int auo_panel_add(struct auo_panel *auo) +{ + struct device *dev= &auo->dsi->dev; + struct device_node *np; + int ret; + + auo->mode = &default_mode; + + auo->supply = devm_regulator_get(dev, "power"); + if (IS_ERR(auo->supply)) + return PTR_ERR(auo->supply); + + auo->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(auo->reset_gpio)) { + dev_err(dev, "cannot get reset-gpios %ld\n", + PTR_ERR(auo->reset_gpio)); + auo->reset_gpio = NULL; + } else { + gpiod_direction_output(auo->reset_gpio, 0); + } + + np = of_parse_phandle(dev->of_node, "backlight", 0); + if (np) { + auo->backlight = of_find_backlight_by_node(np); + of_node_put(np); + + if (!auo->backlight) + return -EPROBE_DEFER; + } + + drm_panel_init(&auo->base); + auo->base.funcs = &auo_panel_funcs; + auo->base.dev = &auo->dsi->dev; + + ret = drm_panel_add(&auo->base); + if (ret < 0) + goto put_backlight; + + return 0; + + put_backlight: + if (auo->backlight) + put_device(&auo->backlight->dev); + + return ret; +} + +static void auo_panel_del(struct auo_panel *auo) +{ + if (auo->base.dev) + drm_panel_remove(&auo->base); + + if (auo->backlight) + put_device(&auo->backlight->dev); +} + +static int auo_panel_probe(struct mipi_dsi_device *dsi) +{ + struct auo_panel *auo; + int ret; + + dsi->lanes = 4; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | + MIPI_DSI_MODE_VIDEO_HSE | + MIPI_DSI_CLOCK_NON_CONTINUOUS | + MIPI_DSI_MODE_EOT_PACKET; + + auo = devm_kzalloc(&dsi->dev, sizeof(*auo), GFP_KERNEL); + if (!auo) { + return -ENOMEM; + } + + mipi_dsi_set_drvdata(dsi, auo); + + auo->dsi = dsi; + + ret = auo_panel_add(auo); + if (ret < 0) { + return ret; + } + + return mipi_dsi_attach(dsi); +} + +static int auo_panel_remove(struct mipi_dsi_device *dsi) +{ + struct auo_panel *auo = mipi_dsi_get_drvdata(dsi); + int ret; + + ret = auo_panel_disable(&auo->base); + if (ret < 0) + dev_err(&dsi->dev, "failed to disable panel: %d\n", ret); + + ret = mipi_dsi_detach(dsi); + if (ret < 0) + dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret); + + drm_panel_detach(&auo->base); + auo_panel_del(auo); + + return 0; +} + +static void auo_panel_shutdown(struct mipi_dsi_device *dsi) +{ + struct auo_panel *auo = mipi_dsi_get_drvdata(dsi); + + auo_panel_disable(&auo->base); +} + +static struct mipi_dsi_driver auo_panel_driver = { + .driver = { + .name = "panel-auo-novatek-1080p", + .of_match_table = auo_of_match, + }, + .probe = auo_panel_probe, + .remove = auo_panel_remove, + .shutdown = auo_panel_shutdown, +}; +module_mipi_dsi_driver(auo_panel_driver); + +MODULE_AUTHOR("Rob Clark robdclark@gmail.com"); +MODULE_DESCRIPTION("AUO Novatek 1080p panel driver"); +MODULE_LICENSE("GPL v2");
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
This is one of several different panels that are used on the Sony Xperia Z3 phone. It can operate in either command or video mode, although so far only video mode is implemented (since that is the mode that the downstream kernel version I happened to be looking at was using, and that is where I got the programming sequences for the panel).
Signed-off-by: Rob Clark robdclark@gmail.com
Couple Notes:
programming sequences and basically everything I know about the panel came from downstream android kernel. I've started a wiki page to document how to translate from downstream kernel-msm way of doing things to upstream panel framework, which may be useful for others wanting to port downstream panel drivers for snapdragon devices:
https://github.com/freedreno/freedreno/wiki/DSI-Panel-Driver-Porting
The Sony phones at least (not sure if this is common) use one of several different panels, with runtime probing. Depending on the device they seem to either use a gpio (simple) or send some DSI commands to read back the panel-id. My rough thinking here about how to handle this is to implement a "panel-meta" driver (or maybe one each for the different probing methods), which would take a list of phandles to the actual candidate panels, and fwd the panel fxn calls to the chosen panel after probing.
.../bindings/panel/auo,novatek-1080p.txt | 25 ++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-auo-novatek-1080p.c | 470 +++++++++++++++++++++ 4 files changed, 505 insertions(+) create mode 100644 Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt create mode 100644 drivers/gpu/drm/panel/panel-auo-novatek-1080p.c
diff --git a/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt new file mode 100644 index 0000000..8a53093 --- /dev/null +++ b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt @@ -0,0 +1,25 @@ +AU Optronics Corporation 1080x1920 DSI panel
+This panel supports both video and command mode (although currently only video +mode is implemented in the driver.
+Required properties: +- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Also, what's the -vid suffix for?
+Optional properties: +- power-supply: phandle of the regulator that provides the supply voltage +- reset-gpio: phandle of gpio for reset line +- backlight: phandle of the backlight device attached to the panel
+Example:
- dsi@54300000 {
panel: panel@0 {compatible = "auo,novatek-1080p-vid";reg = <0>;power-supply = <...>;reset-gpio = <...>;backlight = <...>;};- };
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 6d64c7b..89f0e8c 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -43,4 +43,13 @@ config DRM_PANEL_SHARP_LQ101R1SX01 To compile this driver as a module, choose M here: the module will be called panel-sharp-lq101r1sx01.
+config DRM_PANEL_AUO_NOVATEK_1080P
- tristate "AUO Novatek 1080p video mode panel"
- depends on OF
- depends on DRM_MIPI_DSI
- depends on BACKLIGHT_CLASS_DEVICE
- help
Say Y here if you want to enable support for AUO 1080p DSI panelas found in some Sony XPERIA Z3 devices
Can we sort this alphabetically, please?
endmenu diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 4b2a043..cbcfedf 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o +obj-$(CONFIG_DRM_PANEL_AUO_NOVATEK_1080P) += panel-auo-novatek-1080p.o
This too.
Actually, nevermind. I have local patches to add vendor prefixes more consistently so that we can actually sort properly. I can fix that up in your patch when I apply.
diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c +static int auo_panel_init(struct auo_panel *auo) +{
- struct mipi_dsi_device *dsi = auo->dsi;
- int ret;
- dsi->mode_flags |= MIPI_DSI_MODE_LPM;
- ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2);
I find this notation hard to read. Have you considered moving this into some sort of table that you can loop through? Or perhaps add some helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to help make this more readable?
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0xe0 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xb5, (u8[]){ 0x86 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xb6, (u8[]){ 0x77 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xb8, (u8[]){ 0xad }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x20 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x24 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xc6, (u8[]){ 0x00 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xc5, (u8[]){ 0x32 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0x92, (u8[]){ 0x92 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x10 }, 1);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE,
(u8[]){ 0x03, 0x00 }, 2);- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0x3b, (u8[]){ 0x03, 0x30, 0x06 }, 3);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xbb, (u8[]){ 0x10 }, 1);
- if (ret < 0)
return ret;- msleep(1);
- ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
- if (ret < 0)
return ret;- msleep(30);
- return 0;
+}
+static int auo_panel_on(struct auo_panel *auo) +{
- struct mipi_dsi_device *dsi = auo->dsi;
- int ret;
- dsi->mode_flags |= MIPI_DSI_MODE_LPM;
This is weird.
- ret = mipi_dsi_dcs_set_display_on(dsi);
- if (ret < 0)
return ret;- msleep(40);
- return 0;
+}
+static int auo_panel_off(struct auo_panel *auo) +{
- struct mipi_dsi_device *dsi = auo->dsi;
- int ret;
- dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
And this even more. Doesn't the panel work when you simply send everything in low-power mode?
+static int auo_panel_prepare(struct drm_panel *panel) +{
- struct auo_panel *auo = to_auo_panel(panel);
- int ret;
- if (auo->prepared)
return 0;- DRM_DEBUG("prepare\n");
- if (auo->reset_gpio) {
gpiod_set_value(auo->reset_gpio, 0);msleep(5);- }
- ret = regulator_enable(auo->supply);
- if (ret < 0)
return ret;- msleep(20);
- if (auo->reset_gpio) {
gpiod_set_value(auo->reset_gpio, 1);msleep(10);- }
- msleep(150);
- ret = auo_panel_init(auo);
- if (ret) {
dev_err(panel->dev, "failed to init panel: %d\n", ret);goto poweroff;- }
- ret = auo_panel_on(auo);
- if (ret) {
dev_err(panel->dev, "failed to set panel on: %d\n", ret);goto poweroff;- }
- auo->prepared = true;
- return 0;
+poweroff:
- regulator_disable(auo->supply);
- if (auo->reset_gpio)
gpiod_set_value(auo->reset_gpio, 0);
You should be able to do without the check here, because gpiod_set_value() will simply nop if the GPIO is NULL.
I assume you may not want to do it above because of the additional delays that are only relevant if you have a reset GPIO.
- return ret;
+}
+static int auo_panel_enable(struct drm_panel *panel) +{
- struct auo_panel *auo = to_auo_panel(panel);
- if (auo->enabled)
return 0;- DRM_DEBUG("enable\n");
- if (auo->backlight) {
auo->backlight->props.power = FB_BLANK_UNBLANK;backlight_update_status(auo->backlight);- }
- auo->enabled = true;
- return 0;
+}
+static int auo_panel_add(struct auo_panel *auo) +{
- struct device *dev= &auo->dsi->dev;
- struct device_node *np;
- int ret;
- auo->mode = &default_mode;
This seems to be unused.
- auo->supply = devm_regulator_get(dev, "power");
- if (IS_ERR(auo->supply))
return PTR_ERR(auo->supply);- auo->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(auo->reset_gpio)) {
dev_err(dev, "cannot get reset-gpios %ld\n",PTR_ERR(auo->reset_gpio));auo->reset_gpio = NULL;- } else {
gpiod_direction_output(auo->reset_gpio, 0);
Isn't that what GPIOD_OUT_LOW already does?
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
This is one of several different panels that are used on the Sony Xperia Z3 phone. It can operate in either command or video mode, although so far only video mode is implemented (since that is the mode that the downstream kernel version I happened to be looking at was using, and that is where I got the programming sequences for the panel).
Signed-off-by: Rob Clark robdclark@gmail.com
Couple Notes:
programming sequences and basically everything I know about the panel came from downstream android kernel. I've started a wiki page to document how to translate from downstream kernel-msm way of doing things to upstream panel framework, which may be useful for others wanting to port downstream panel drivers for snapdragon devices:
https://github.com/freedreno/freedreno/wiki/DSI-Panel-Driver-Porting
The Sony phones at least (not sure if this is common) use one of several different panels, with runtime probing. Depending on the device they seem to either use a gpio (simple) or send some DSI commands to read back the panel-id. My rough thinking here about how to handle this is to implement a "panel-meta" driver (or maybe one each for the different probing methods), which would take a list of phandles to the actual candidate panels, and fwd the panel fxn calls to the chosen panel after probing.
.../bindings/panel/auo,novatek-1080p.txt | 25 ++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-auo-novatek-1080p.c | 470 +++++++++++++++++++++ 4 files changed, 505 insertions(+) create mode 100644 Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt create mode 100644 drivers/gpu/drm/panel/panel-auo-novatek-1080p.c
diff --git a/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt new file mode 100644 index 0000000..8a53093 --- /dev/null +++ b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt @@ -0,0 +1,25 @@ +AU Optronics Corporation 1080x1920 DSI panel
+This panel supports both video and command mode (although currently only video +mode is implemented in the driver.
+Required properties: +- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Maybe Sony or someone else can chime in? That somewhat generic name was all I could get from downstream android kernel. I'm sure there is a better possible name, although I have no means to find that out myself.
Also, what's the -vid suffix for?
the same panel seems to also work in cmd mode.. so idea was to have -vid and -cmd compat strings to choose which mode to operate in.
+Optional properties: +- power-supply: phandle of the regulator that provides the supply voltage +- reset-gpio: phandle of gpio for reset line +- backlight: phandle of the backlight device attached to the panel
+Example:
dsi@54300000 {panel: panel@0 {compatible = "auo,novatek-1080p-vid";reg = <0>;power-supply = <...>;reset-gpio = <...>;backlight = <...>;};};diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 6d64c7b..89f0e8c 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -43,4 +43,13 @@ config DRM_PANEL_SHARP_LQ101R1SX01 To compile this driver as a module, choose M here: the module will be called panel-sharp-lq101r1sx01.
+config DRM_PANEL_AUO_NOVATEK_1080P
tristate "AUO Novatek 1080p video mode panel"depends on OFdepends on DRM_MIPI_DSIdepends on BACKLIGHT_CLASS_DEVICEhelpSay Y here if you want to enable support for AUO 1080p DSI panelas found in some Sony XPERIA Z3 devicesCan we sort this alphabetically, please?
endmenu diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 4b2a043..cbcfedf 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o +obj-$(CONFIG_DRM_PANEL_AUO_NOVATEK_1080P) += panel-auo-novatek-1080p.o
This too.
Actually, nevermind. I have local patches to add vendor prefixes more consistently so that we can actually sort properly. I can fix that up in your patch when I apply.
diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c +static int auo_panel_init(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags |= MIPI_DSI_MODE_LPM;ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2);I find this notation hard to read. Have you considered moving this into some sort of table that you can loop through? Or perhaps add some helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to help make this more readable?
Yeah, helper macro thing might be a reasonable idea. The table option makes it hard to use the helpers for things that are not non-standard, or when you need delays, etc..
if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0xe0 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xb5, (u8[]){ 0x86 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xb6, (u8[]){ 0x77 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xb8, (u8[]){ 0xad }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x20 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_enter_sleep_mode(dsi);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x24 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xfb, (u8[]){ 0x01 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xc6, (u8[]){ 0x00 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xc5, (u8[]){ 0x32 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0x92, (u8[]){ 0x92 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xff, (u8[]){ 0x10 }, 1);if (ret < 0)return ret;ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE,(u8[]){ 0x03, 0x00 }, 2);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0x3b, (u8[]){ 0x03, 0x30, 0x06 }, 3);if (ret < 0)return ret;ret = mipi_dsi_dcs_write(dsi, 0xbb, (u8[]){ 0x10 }, 1);if (ret < 0)return ret;msleep(1);ret = mipi_dsi_dcs_exit_sleep_mode(dsi);if (ret < 0)return ret;msleep(30);return 0;+}
+static int auo_panel_on(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags |= MIPI_DSI_MODE_LPM;This is weird.
ret = mipi_dsi_dcs_set_display_on(dsi);if (ret < 0)return ret;msleep(40);return 0;+}
+static int auo_panel_off(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;And this even more. Doesn't the panel work when you simply send everything in low-power mode?
I wouldn't expect low power mode to have enough bandwidth for the video signal.. but otoh it seems like I need to use lpm for power-on/off sequence. Maybe we should wrap that up in a helper to enable/disable lpm? But that seemed a bit overkill.
+static int auo_panel_prepare(struct drm_panel *panel) +{
struct auo_panel *auo = to_auo_panel(panel);int ret;if (auo->prepared)return 0;DRM_DEBUG("prepare\n");if (auo->reset_gpio) {gpiod_set_value(auo->reset_gpio, 0);msleep(5);}ret = regulator_enable(auo->supply);if (ret < 0)return ret;msleep(20);if (auo->reset_gpio) {gpiod_set_value(auo->reset_gpio, 1);msleep(10);}msleep(150);ret = auo_panel_init(auo);if (ret) {dev_err(panel->dev, "failed to init panel: %d\n", ret);goto poweroff;}ret = auo_panel_on(auo);if (ret) {dev_err(panel->dev, "failed to set panel on: %d\n", ret);goto poweroff;}auo->prepared = true;return 0;+poweroff:
regulator_disable(auo->supply);if (auo->reset_gpio)gpiod_set_value(auo->reset_gpio, 0);You should be able to do without the check here, because gpiod_set_value() will simply nop if the GPIO is NULL.
ok
I assume you may not want to do it above because of the additional delays that are only relevant if you have a reset GPIO.
return ret;+}
+static int auo_panel_enable(struct drm_panel *panel) +{
struct auo_panel *auo = to_auo_panel(panel);if (auo->enabled)return 0;DRM_DEBUG("enable\n");if (auo->backlight) {auo->backlight->props.power = FB_BLANK_UNBLANK;backlight_update_status(auo->backlight);}auo->enabled = true;return 0;+}
+static int auo_panel_add(struct auo_panel *auo) +{
struct device *dev= &auo->dsi->dev;struct device_node *np;int ret;auo->mode = &default_mode;This seems to be unused.
yeah, I think just cargo cult'd from sharp panel..
auo->supply = devm_regulator_get(dev, "power");if (IS_ERR(auo->supply))return PTR_ERR(auo->supply);auo->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);if (IS_ERR(auo->reset_gpio)) {dev_err(dev, "cannot get reset-gpios %ld\n",PTR_ERR(auo->reset_gpio));auo->reset_gpio = NULL;} else {gpiod_direction_output(auo->reset_gpio, 0);Isn't that what GPIOD_OUT_LOW already does?
hmm, maybe?
BR, -R
On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote:
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[..]
+- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Maybe Sony or someone else can chime in? That somewhat generic name was all I could get from downstream android kernel. I'm sure there is a better possible name, although I have no means to find that out myself.
We're working on it.
Also, what's the -vid suffix for?
the same panel seems to also work in cmd mode.. so idea was to have -vid and -cmd compat strings to choose which mode to operate in.
An alternative would be to make it a bool property, to indicate video mode - following how the framework is implemented.
[..]
diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c +static int auo_panel_init(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags |= MIPI_DSI_MODE_LPM;ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2);I find this notation hard to read. Have you considered moving this into some sort of table that you can loop through? Or perhaps add some helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to help make this more readable?
Yeah, helper macro thing might be a reasonable idea. The table option makes it hard to use the helpers for things that are not non-standard, or when you need delays, etc..
I agree with you here, we don't want lists of data that the driver has to interpret into writes (of various types) and delays.
if (ret < 0)return ret;
Regards, Bjorn
On Mon, Aug 10, 2015 at 3:54 PM, Bjorn Andersson bjorn.andersson@sonymobile.com wrote:
On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote:
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[..]
+- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Maybe Sony or someone else can chime in? That somewhat generic name was all I could get from downstream android kernel. I'm sure there is a better possible name, although I have no means to find that out myself.
We're working on it.
Cool, thx
Also, what's the -vid suffix for?
the same panel seems to also work in cmd mode.. so idea was to have -vid and -cmd compat strings to choose which mode to operate in.
An alternative would be to make it a bool property, to indicate video mode - following how the framework is implemented.
I was debating both approaches.. I think I ended up going with the compat name so that we could, if wanted, have the two split out into different drivers, depending on how much was in common. Not even really sure if that is worth worrying about or not.
BR, -R
[..]
diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c +static int auo_panel_init(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags |= MIPI_DSI_MODE_LPM;ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2);I find this notation hard to read. Have you considered moving this into some sort of table that you can loop through? Or perhaps add some helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to help make this more readable?
Yeah, helper macro thing might be a reasonable idea. The table option makes it hard to use the helpers for things that are not non-standard, or when you need delays, etc..
I agree with you here, we don't want lists of data that the driver has to interpret into writes (of various types) and delays.
if (ret < 0)return ret;Regards, Bjorn
On Mon, Aug 10, 2015 at 12:54:20PM -0700, Bjorn Andersson wrote:
On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote:
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[..]
+- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Maybe Sony or someone else can chime in? That somewhat generic name was all I could get from downstream android kernel. I'm sure there is a better possible name, although I have no means to find that out myself.
We're working on it.
Also, what's the -vid suffix for?
the same panel seems to also work in cmd mode.. so idea was to have -vid and -cmd compat strings to choose which mode to operate in.
An alternative would be to make it a bool property, to indicate video mode - following how the framework is implemented.
Please, let's not do either. This doesn't belong in the compatible string. The compatible string specifies the panel, and the panel supports both video and command modes. That's implied by the compatible string.
Which mode to use is a configuration or policy decision and therefore doesn't belong in device tree. It should be up to the display driver to determine what the preferred mode of operation is.
Thierry
On Mon, Aug 17, 2015 at 7:38 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Mon, Aug 10, 2015 at 12:54:20PM -0700, Bjorn Andersson wrote:
On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote:
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[..]
+- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Maybe Sony or someone else can chime in? That somewhat generic name was all I could get from downstream android kernel. I'm sure there is a better possible name, although I have no means to find that out myself.
We're working on it.
Also, what's the -vid suffix for?
the same panel seems to also work in cmd mode.. so idea was to have -vid and -cmd compat strings to choose which mode to operate in.
An alternative would be to make it a bool property, to indicate video mode - following how the framework is implemented.
Please, let's not do either. This doesn't belong in the compatible string. The compatible string specifies the panel, and the panel supports both video and command modes. That's implied by the compatible string.
Which mode to use is a configuration or policy decision and therefore doesn't belong in device tree. It should be up to the display driver to determine what the preferred mode of operation is.
I would call it a "system integrator decision".. and at least currently we don't have anywhere better than DT for that. Maybe it's one of those "if all you have is a hammer, everything looks like a nail" things..
BR, -R
Thierry
On Mon, Aug 17, 2015 at 10:48:20AM -0400, Rob Clark wrote:
On Mon, Aug 17, 2015 at 7:38 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Mon, Aug 10, 2015 at 12:54:20PM -0700, Bjorn Andersson wrote:
On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote:
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[..]
+- compatible: should be "auo,novatek-1080p-vid"
This looks a little generic for a compatible string. Can't we get at the specific panel model number that's been used? What if AUO ever produced some other Novatek panel with a 1080p resolution?
Maybe Sony or someone else can chime in? That somewhat generic name was all I could get from downstream android kernel. I'm sure there is a better possible name, although I have no means to find that out myself.
We're working on it.
Also, what's the -vid suffix for?
the same panel seems to also work in cmd mode.. so idea was to have -vid and -cmd compat strings to choose which mode to operate in.
An alternative would be to make it a bool property, to indicate video mode - following how the framework is implemented.
Please, let's not do either. This doesn't belong in the compatible string. The compatible string specifies the panel, and the panel supports both video and command modes. That's implied by the compatible string.
Which mode to use is a configuration or policy decision and therefore doesn't belong in device tree. It should be up to the display driver to determine what the preferred mode of operation is.
I would call it a "system integrator decision".. and at least currently we don't have anywhere better than DT for that. Maybe it's one of those "if all you have is a hammer, everything looks like a nail" things..
If it were a system integrator decision then I'd agree it should be parameterizable in DT. But to my knowledge you can run any command mode capable display in video mode just fine.
Thierry
On Fri, Aug 07, 2015 at 12:11:31PM -0400, Rob Clark wrote:
On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[...]
+static int auo_panel_on(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags |= MIPI_DSI_MODE_LPM;This is weird.
ret = mipi_dsi_dcs_set_display_on(dsi);if (ret < 0)return ret;msleep(40);return 0;+}
+static int auo_panel_off(struct auo_panel *auo) +{
struct mipi_dsi_device *dsi = auo->dsi;int ret;dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;And this even more. Doesn't the panel work when you simply send everything in low-power mode?
I wouldn't expect low power mode to have enough bandwidth for the video signal.. but otoh it seems like I need to use lpm for power-on/off sequence. Maybe we should wrap that up in a helper to enable/disable lpm? But that seemed a bit overkill.
I think there's a misunderstanding, which arguable might stem from a lack of documentation. The intention for MIPI_DSI_MODE_LPM was to be used in conjunction with "host-driven" command mode.
Perhaps I should elaborate on the vocabulary here: Tegra supports two types of command mode: "host-driven" and "DC-driven". Host driven command mode is used to perform panel setup (using DCS and vendor- specific commands). "DC-driven" command mode is used to update the framebuffer using write_memory_start and write_memory_continue DCS commands directly generated by the DSI controller.
In the latter case you'd obviously want to run in high-speed mode to achieve the throughput necessary to drive you panel at the requested resolution and framerate. In the former your device should be able to receive command in both high speed and low power modes. However some hardware is known not to work with high speed command transmission. MIPI_DSI_MODE_LPM is targetted at these cases, so that display drivers know not to attempt high-speed transmission of initial command packets.
Note how MIPI_DSI_MODE_LPM translates to MIPI_DSI_MSG_USE_LPM when transferring messages (see mipi_dsi_device_transfer()). Looking at the comment for the MIPI_DSI_MODE_LPM definition I realize that it isn't very precise, but I have trouble coming up with anything better. Perhaps:
/* transmit command messages (non-video data) in low power mode */ #define MIPI_DSI_MODE_LPM BIT(11)
Any ideas?
On a semi-related note, some of the other flags are rather badly documented. I do see that both Exynos and MSM implement most of these (specifically the MIPI_DSI_MODE_VIDEO_H* ones), perhaps the comments for all of those should be revisited. Ideally they'd be annotated with a reference to the spec, like we do for MIPI_DSI_CLOCK_NON_CONTINUOUS.
Thierry
On Tue 21 Jul 12:36 PDT 2015, Rob Clark wrote:
[..]
+++ b/Documentation/devicetree/bindings/panel/auo,novatek-1080p.txt @@ -0,0 +1,25 @@ +AU Optronics Corporation 1080x1920 DSI panel
+This panel supports both video and command mode (although currently only video +mode is implemented in the driver.
+Required properties: +- compatible: should be "auo,novatek-1080p-vid"
The panel name is AUO H515DAN02.0
+Optional properties: +- power-supply: phandle of the regulator that provides the supply voltage +- reset-gpio: phandle of gpio for reset line +- backlight: phandle of the backlight device attached to the panel
[..]
diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c
[..]
+static int auo_panel_init(struct auo_panel *auo) +{
[..]
- ret = mipi_dsi_dcs_write(dsi, 0x3b, (u8[]){ 0x03, 0x30, 0x06 }, 3);
- if (ret < 0)
return ret;- ret = mipi_dsi_dcs_write(dsi, 0xbb, (u8[]){ 0x10 }, 1);
This should be 0x3 for video mode and 0x10 for command mode.
- if (ret < 0)
return ret;- msleep(1);
- ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
- if (ret < 0)
return ret;- msleep(30);
- return 0;
+}
Regards, Bjorn
dri-devel@lists.freedesktop.org
-
Bjorn Andersson -
Rob Clark -
Thierry Reding