diff mbox

drm/panel: auo novatek 1080p video mode panel

Message ID 1437507362-20162-1-git-send-email-robdclark@gmail.com (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Rob Clark July 21, 2015, 7:36 p.m. UTC
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

Comments

Thierry Reding Aug. 7, 2015, 1:19 p.m. UTC | #1
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:
>  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@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
> +

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?
Rob Clark Aug. 7, 2015, 4:11 p.m. UTC | #2
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:
>>  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?

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 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?
>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Andersson Aug. 10, 2015, 7:54 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Clark Aug. 10, 2015, 9:45 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Aug. 17, 2015, 11:38 a.m. UTC | #5
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
Thierry Reding Aug. 17, 2015, 12:07 p.m. UTC | #6
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
Rob Clark Aug. 17, 2015, 2:48 p.m. UTC | #7
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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Aug. 17, 2015, 2:57 p.m. UTC | #8
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
Bjorn Andersson Aug. 17, 2015, 5:27 p.m. UTC | #9
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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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");