diff mbox series

drm/panel: s6e63m0: Order enable/disable sequence

Message ID 20200817213906.88207-1-linus.walleij@linaro.org
State Accepted
Commit ec219f1ba23777c84e913b08e192413eea22d220
Headers show
Series drm/panel: s6e63m0: Order enable/disable sequence | expand

Commit Message

Linus Walleij Aug. 17, 2020, 9:39 p.m. UTC
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@gmail.com>
Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
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(-)

Comments

Sam Ravnborg Aug. 18, 2020, 5:33 p.m. UTC | #1
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@gmail.com>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Browsed through this patch - looks fine.
Acked-by: Sam Ravnborg <sam@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

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