[PATCH] drm/panel: s6e63m0: Order enable/disable sequence

[Sam Ravnborg]

Hi Linus.

On Mon, Aug 17, 2020 at 11:39:06PM +0200, Linus Walleij wrote:
> The upstream S6E63M0 driver has some pecularities around
> the prepare/enable disable/unprepare sequence: the screen
> is taken out of sleep in prepare() as part of
> s6e63m0_init() the put to on with MIPI_DCS_SET_DISPLAY_ON
> in enable().
> 
> However it is just put into sleep mode directly in
> disable(). As disable()/enable() can be called without
> unprepare()/prepare() being called, this is unbalanced,
> we should take the display out of sleep in enable()
> then turn it off().
> 
> Further MIPI_DCS_SET_DISPLAY_OFF is never called
> balanced with MIPI_DCS_SET_DISPLAY_ON.
> 
> The vendor driver for Samsung GT-I8190 (Golden) does all
> of these things in strict order.
> 
> Augment the driver to do exit sleep/set display on in
> enable() and set display off/enter sleep in disable().
> 
> Further send an explict reset pulse in power_on() so we
> come up in a known state, and issue the MCS_ERROR_CHECK
> command after setting display on like the vendor driver
> does. Also use the timings from the vendor driver in
> the sequence.
> 
> Doing all of these things makes the display much more
> stable on the Samsung GT-I8190 when enabling/disabling
> the display pipeline.
> 
> Cc: Paweł Chmiel <pawel.mikolaj.chmiel at gmail.com>
> Cc: Stephan Gerhold <stephan at gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>

Browsed through this patch - looks fine.
Acked-by: Sam Ravnborg <sam at ravnborg.org>

	Sam

> ---
> This is based on top of the earlier patches for s6e63m0.
> ---
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
> index f1d96ec3b57f..49b0470bcccd 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
> @@ -26,6 +26,7 @@
>  #define MCS_ELVSS_ON                0xb1
>  #define MCS_MIECTL1                0xc0
>  #define MCS_BCMODE                              0xc1
> +#define MCS_ERROR_CHECK		0xd5
>  #define MCS_READ_ID1		0xda
>  #define MCS_READ_ID2		0xdb
>  #define MCS_READ_ID3		0xdc
> @@ -281,8 +282,6 @@ static void s6e63m0_init(struct s6e63m0 *ctx)
>  
>  	s6e63m0_dcs_write_seq_static(ctx, MCS_ELVSS_ON,
>  				     0x0b);
> -
> -	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
>  }
>  
>  static int s6e63m0_power_on(struct s6e63m0 *ctx)
> @@ -295,6 +294,9 @@ static int s6e63m0_power_on(struct s6e63m0 *ctx)
>  
>  	msleep(25);
>  
> +	/* Be sure to send a reset pulse */
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	msleep(5);
>  	gpiod_set_value(ctx->reset_gpio, 0);
>  	msleep(120);
>  
> @@ -324,8 +326,10 @@ static int s6e63m0_disable(struct drm_panel *panel)
>  
>  	backlight_disable(ctx->bl_dev);
>  
> +	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_OFF);
> +	msleep(10);
>  	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
> -	msleep(200);
> +	msleep(120);
>  
>  	ctx->enabled = false;
>  
> @@ -391,7 +395,15 @@ static int s6e63m0_enable(struct drm_panel *panel)
>  	if (ctx->enabled)
>  		return 0;
>  
> +	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
> +	msleep(120);
>  	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
> +	msleep(10);
> +
> +	s6e63m0_dcs_write_seq_static(ctx, MCS_ERROR_CHECK,
> +				     0xE7, 0x14, 0x60, 0x17, 0x0A, 0x49, 0xC3,
> +				     0x8F, 0x19, 0x64, 0x91, 0x84, 0x76, 0x20,
> +				     0x0F, 0x00);
>  
>  	backlight_enable(ctx->bl_dev);
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel