[PATCH] drm/panel: auo novatek 1080p video mode panel
Thierry Reding
thierry.reding at gmail.com
Fri Aug 7 06:19:33 PDT 2015
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 at 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"
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 at 54300000 {
> + panel: panel at 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
> +
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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150807/65b96da6/attachment.sig>
More information about the dri-devel
mailing list