diff mbox

[V6,2/8] drm/panel: Add support for prepare and unprepare routines

Message ID 1406316130-4744-3-git-send-email-ajaykumar.rs@samsung.com (mailing list archive)
State Accepted
Headers show

Commit Message

Ajay Kumar July 25, 2014, 7:22 p.m. UTC
Now that we have 2 new callbacks(prepare and unprepare) for drm_panel,
make changes in all the drm drivers which use the drm_panel framework
to support the new callbacks.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dpi.c |    8 +++--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c |    7 +++++
 drivers/gpu/drm/panel/panel-ld9040.c    |   18 +++++++++--
 drivers/gpu/drm/panel/panel-s6e8aa0.c   |   18 +++++++++--
 drivers/gpu/drm/panel/panel-simple.c    |   51 ++++++++++++++++++++++++-------
 drivers/gpu/drm/tegra/output.c          |    2 ++
 6 files changed, 85 insertions(+), 19 deletions(-)

Comments

Thierry Reding July 30, 2014, 10:32 a.m. UTC | #1
On Sat, Jul 26, 2014 at 12:52:04AM +0530, Ajay Kumar wrote:
> Now that we have 2 new callbacks(prepare and unprepare) for drm_panel,
> make changes in all the drm drivers which use the drm_panel framework
> to support the new callbacks.
> 
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c |    8 +++--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c |    7 +++++
>  drivers/gpu/drm/panel/panel-ld9040.c    |   18 +++++++++--
>  drivers/gpu/drm/panel/panel-s6e8aa0.c   |   18 +++++++++--
>  drivers/gpu/drm/panel/panel-simple.c    |   51 ++++++++++++++++++++++++-------
>  drivers/gpu/drm/tegra/output.c          |    2 ++
>  6 files changed, 85 insertions(+), 19 deletions(-)

I'd prefer for this to be split up into patches per panel and per
display driver. The reason is that if this patch is merged as-is, then
if it's ever determined to cause a regression it'll be difficult to find
out which change exactly caused this.

But to not break bisectability you'll need to be careful in how you
break up the patches. I think the following should work:

	- for each panel driver, implement dummy .prepare() and
	  .unprepare() that return 0
	- for each display driver, make use of drm_panel_prepare() and
	  drm_panel_unprepare()
	- for each panel driver, properly implement .prepare() and
	  .unprepare() (presumably by moving code out of .enable() and
	  .disable(), respectively)

I have a couple more comments below about the ordering of the .prepare()
vs. .enable() and .disable() vs. .unprepare() calls.

> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 5e78d45..2f58e45 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1333,6 +1333,12 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = drm_panel_prepare(dsi->panel);
> +	if (ret < 0) {
> +		exynos_dsi_poweroff(dsi);
> +		return ret;
> +	}
> +
>  	ret = drm_panel_enable(dsi->panel);
>  	if (ret < 0) {
>  		exynos_dsi_poweroff(dsi);
> @@ -1354,6 +1360,7 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
>  
>  	exynos_dsi_set_display_enable(dsi, false);
>  	drm_panel_disable(dsi->panel);
> +	drm_panel_unprepare(dsi->panel);
>  	exynos_dsi_poweroff(dsi);

I don't know Exynos very well, so this may be completely wrong, but
should disable_panel_prepare() be called much earlier than
drm_panel_enable() and drm_panel_unprepare() much later than
drm_panel_disable()? With the above the result is still the same, so
you'll get the same glitches as without their separation.

Without knowing exactly what Exynos does in the above, I'd expect the
correct sequence to be something like this:

	ret = exynos_dsi_power_on(dsi);
	if (ret < 0)
		return ret;

	ret = drm_panel_prepare(dsi->panel);
	if (ret < 0) {
		exynos_dsi_poweroff(dsi);
		return ret;
	}

	exynos_dsi_set_display_mode(dsi);
	exynos_dsi_set_display_enable(dsi, true);

	ret = drm_panel_enable(dsi->panel);
	if (ret < 0) {
		/* perhaps undo exynos_dsi_set_display_enable() here? */
		exynos_dsi_poweroff(dsi);
		return ret;
	}

And for disable:

	drm_panel_disable(dsi->panel);
	exynos_dsi_set_display_enable(dsi, false);
	drm_panel_unprepare(dsi->panel);
	exynos_dsi_poweroff(dsi);

Perhaps I should quote the kerneldoc that I added for drm_panel_funcs to
underline why I think this is important:

/**
 * struct drm_panel_funcs - perform operations on a given panel
 * @disable: disable panel (turn off back light, etc.)
 * @unprepare: turn off panel
 * @prepare: turn on panel and perform set up
 * @enable: enable panel (turn on back light, etc.)
 * @get_modes: add modes to the connector that the panel is attached to and
 * return the number of modes added
 *
 * The .prepare() function is typically called before the display controller
 * starts to transmit video data. Panel drivers can use this to turn the panel
 * on and wait for it to become ready. If additional configuration is required
 * (via a control bus such as I2C, SPI or DSI for example) this is a good time
 * to do that.
 *
 * After the display controller has started transmitting video data, it's safe
 * to call the .enable() function. This will typically enable the backlight to
 * make the image on screen visible. Some panels require a certain amount of
 * time or frames before the image is displayed. This function is responsible
 * for taking this into account before enabling the backlight to avoid visual
 * glitches.
 *
 * Before stopping video transmission from the display controller it can be
 * necessary to turn off the panel to avoid visual glitches. This is done in
 * the .disable() function. Analogously to .enable() this typically involves
 * turning off the backlight and waiting for some time to make sure no image
 * is visible on the panel. It is then safe for the display controller to
 * cease transmission of video data.
 *
 * To save power when no video data is transmitted, a driver can power down
 * the panel. This is the job of the .unprepare() function.
 */

As you see, .prepare() should do whatever is necessary to make the panel
accept a stream of video data, then the display driver can start sending
video data and call .enable() to make the transmitted data visible.

Analogously .disable() should turn off the display, so that the user can
no longer see what's going on, then the display controller can cease
transmission of video data (and since the panel is disabled this should
no longer cause visual glitches) and then call .unprepare() to turn the
panel off.

I know that this isn't immediately relevant just yet because currently
the backlight will already turn on by default, but like we discussed
earlier I have ideas on how to properly fix it. As a matter of fact I'll
go and send out another mail about that when I'm through this series.

>  static const struct drm_panel_funcs ld9040_drm_funcs = {
> +	.unprepare = ld9040_unprepare,
>  	.disable = ld9040_disable,
> +	.prepare = ld9040_prepare,
>  	.enable = ld9040_enable,
>  	.get_modes = ld9040_get_modes,
>  };

The patch I applied for .prepare() and .unprepare() I've reordered the
functions slightly to give a more natural sequence. .disable() is now
first, while .unprepare() is next, since that's the sequence that they
should be called in.

Patches for the panel drivers should follow this same ordering.

> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index a251361..fb0cfe2 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -45,7 +45,8 @@ struct panel_desc {
>  
>  struct panel_simple {
>  	struct drm_panel base;
> -	bool enabled;
> +	bool panel_enabled;
> +	bool backlight_enabled;

Perhaps this should rather be:

	bool prepared;
	bool enabled;

?

> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index 446837e..b574ee6 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -139,9 +139,11 @@ static void tegra_encoder_dpms(struct drm_encoder *encoder, int mode)
>  
>  	if (mode != DRM_MODE_DPMS_ON) {
>  		drm_panel_disable(panel);
> +		drm_panel_unprepare(panel);
>  		tegra_output_disable(output);

Similarly to my comments for the Exynos drivers, this should be:

		drm_panel_disable(panel);
		tegra_output_disable(output);
		drm_panel_unprepare(panel);

>  	} else {
>  		tegra_output_enable(output);
> +		drm_panel_prepare(panel);
>  		drm_panel_enable(panel);
>  	}

and

		drm_panel_prepare(panel);
		tegra_output_enable(output);
		drm_panel_enable(panel);

Thierry
Ajay kumar July 30, 2014, 11:09 a.m. UTC | #2
On Wed, Jul 30, 2014 at 4:02 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sat, Jul 26, 2014 at 12:52:04AM +0530, Ajay Kumar wrote:
>> Now that we have 2 new callbacks(prepare and unprepare) for drm_panel,
>> make changes in all the drm drivers which use the drm_panel framework
>> to support the new callbacks.
>>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c |    8 +++--
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c |    7 +++++
>>  drivers/gpu/drm/panel/panel-ld9040.c    |   18 +++++++++--
>>  drivers/gpu/drm/panel/panel-s6e8aa0.c   |   18 +++++++++--
>>  drivers/gpu/drm/panel/panel-simple.c    |   51 ++++++++++++++++++++++++-------
>>  drivers/gpu/drm/tegra/output.c          |    2 ++
>>  6 files changed, 85 insertions(+), 19 deletions(-)
>
> I'd prefer for this to be split up into patches per panel and per
> display driver. The reason is that if this patch is merged as-is, then
> if it's ever determined to cause a regression it'll be difficult to find
> out which change exactly caused this.
>
> But to not break bisectability you'll need to be careful in how you
> break up the patches. I think the following should work:
>
>         - for each panel driver, implement dummy .prepare() and
>           .unprepare() that return 0
>         - for each display driver, make use of drm_panel_prepare() and
>           drm_panel_unprepare()
>         - for each panel driver, properly implement .prepare() and
>           .unprepare() (presumably by moving code out of .enable() and
>           .disable(), respectively)
>
I will try this. With this approach, compilation should not fail.

> I have a couple more comments below about the ordering of the .prepare()
> vs. .enable() and .disable() vs. .unprepare() calls.
>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 5e78d45..2f58e45 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1333,6 +1333,12 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
>>       if (ret < 0)
>>               return ret;
>>
>> +     ret = drm_panel_prepare(dsi->panel);
>> +     if (ret < 0) {
>> +             exynos_dsi_poweroff(dsi);
>> +             return ret;
>> +     }
>> +
>>       ret = drm_panel_enable(dsi->panel);
>>       if (ret < 0) {
>>               exynos_dsi_poweroff(dsi);
>> @@ -1354,6 +1360,7 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
>>
>>       exynos_dsi_set_display_enable(dsi, false);
>>       drm_panel_disable(dsi->panel);
>> +     drm_panel_unprepare(dsi->panel);
>>       exynos_dsi_poweroff(dsi);
>
> I don't know Exynos very well, so this may be completely wrong, but
> should disable_panel_prepare() be called much earlier than
> drm_panel_enable() and drm_panel_unprepare() much later than
> drm_panel_disable()? With the above the result is still the same, so
> you'll get the same glitches as without their separation.
Actually, I have not worked on DSI panels.
And till now, these DSI panels have been working with just the
enable/disable callback. So, I thought it might not really cause a
glitch/garbage on the screen.
I just placed the new panel calls so that basic working will not be broken.
It would be great if someone can test this on exynos boards with DSI panels :(
Inki/Andrej?

Anyways, now there is a kerneldoc which explains all these calls,
I will rearrange the panel calls.

> Without knowing exactly what Exynos does in the above, I'd expect the
> correct sequence to be something like this:
>
>         ret = exynos_dsi_power_on(dsi);
>         if (ret < 0)
>                 return ret;
>
>         ret = drm_panel_prepare(dsi->panel);
>         if (ret < 0) {
>                 exynos_dsi_poweroff(dsi);
>                 return ret;
>         }
>
>         exynos_dsi_set_display_mode(dsi);
>         exynos_dsi_set_display_enable(dsi, true);
>
>         ret = drm_panel_enable(dsi->panel);
>         if (ret < 0) {
>                 /* perhaps undo exynos_dsi_set_display_enable() here? */
>                 exynos_dsi_poweroff(dsi);
>                 return ret;
>         }
>
> And for disable:
>
>         drm_panel_disable(dsi->panel);
>         exynos_dsi_set_display_enable(dsi, false);
>         drm_panel_unprepare(dsi->panel);
>         exynos_dsi_poweroff(dsi);
>
> Perhaps I should quote the kerneldoc that I added for drm_panel_funcs to
> underline why I think this is important:
>
> /**
>  * struct drm_panel_funcs - perform operations on a given panel
>  * @disable: disable panel (turn off back light, etc.)
>  * @unprepare: turn off panel
>  * @prepare: turn on panel and perform set up
>  * @enable: enable panel (turn on back light, etc.)
>  * @get_modes: add modes to the connector that the panel is attached to and
>  * return the number of modes added
>  *
>  * The .prepare() function is typically called before the display controller
>  * starts to transmit video data. Panel drivers can use this to turn the panel
>  * on and wait for it to become ready. If additional configuration is required
>  * (via a control bus such as I2C, SPI or DSI for example) this is a good time
>  * to do that.
>  *
>  * After the display controller has started transmitting video data, it's safe
>  * to call the .enable() function. This will typically enable the backlight to
>  * make the image on screen visible. Some panels require a certain amount of
>  * time or frames before the image is displayed. This function is responsible
>  * for taking this into account before enabling the backlight to avoid visual
>  * glitches.
>  *
>  * Before stopping video transmission from the display controller it can be
>  * necessary to turn off the panel to avoid visual glitches. This is done in
>  * the .disable() function. Analogously to .enable() this typically involves
>  * turning off the backlight and waiting for some time to make sure no image
>  * is visible on the panel. It is then safe for the display controller to
>  * cease transmission of video data.
>  *
>  * To save power when no video data is transmitted, a driver can power down
>  * the panel. This is the job of the .unprepare() function.
>  */
>
> As you see, .prepare() should do whatever is necessary to make the panel
> accept a stream of video data, then the display driver can start sending
> video data and call .enable() to make the transmitted data visible.
>
> Analogously .disable() should turn off the display, so that the user can
> no longer see what's going on, then the display controller can cease
> transmission of video data (and since the panel is disabled this should
> no longer cause visual glitches) and then call .unprepare() to turn the
> panel off.
>
> I know that this isn't immediately relevant just yet because currently
> the backlight will already turn on by default, but like we discussed
> earlier I have ideas on how to properly fix it. As a matter of fact I'll
> go and send out another mail about that when I'm through this series.
>
>>  static const struct drm_panel_funcs ld9040_drm_funcs = {
>> +     .unprepare = ld9040_unprepare,
>>       .disable = ld9040_disable,
>> +     .prepare = ld9040_prepare,
>>       .enable = ld9040_enable,
>>       .get_modes = ld9040_get_modes,
>>  };
>
> The patch I applied for .prepare() and .unprepare() I've reordered the
> functions slightly to give a more natural sequence. .disable() is now
> first, while .unprepare() is next, since that's the sequence that they
> should be called in.
>
> Patches for the panel drivers should follow this same ordering.
Ok. I will follow the same order for all panel drivers.

>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>> index a251361..fb0cfe2 100644
>> --- a/drivers/gpu/drm/panel/panel-simple.c
>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>> @@ -45,7 +45,8 @@ struct panel_desc {
>>
>>  struct panel_simple {
>>       struct drm_panel base;
>> -     bool enabled;
>> +     bool panel_enabled;
>> +     bool backlight_enabled;
>
> Perhaps this should rather be:
>
>         bool prepared;
>         bool enabled;
>
More generic. Will change it!

Ajay

>
>> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
>> index 446837e..b574ee6 100644
>> --- a/drivers/gpu/drm/tegra/output.c
>> +++ b/drivers/gpu/drm/tegra/output.c
>> @@ -139,9 +139,11 @@ static void tegra_encoder_dpms(struct drm_encoder *encoder, int mode)
>>
>>       if (mode != DRM_MODE_DPMS_ON) {
>>               drm_panel_disable(panel);
>> +             drm_panel_unprepare(panel);
>>               tegra_output_disable(output);
>
> Similarly to my comments for the Exynos drivers, this should be:
>
>                 drm_panel_disable(panel);
>                 tegra_output_disable(output);
>                 drm_panel_unprepare(panel);
>
>>       } else {
>>               tegra_output_enable(output);
>> +             drm_panel_prepare(panel);
>>               drm_panel_enable(panel);
>>       }
>
> and
>
>                 drm_panel_prepare(panel);
>                 tegra_output_enable(output);
>                 drm_panel_enable(panel);
>
> Thierry
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 3aa1c7e..fa08f05 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -125,14 +125,18 @@  static int exynos_dpi_create_connector(struct exynos_drm_display *display,
 
 static void exynos_dpi_poweron(struct exynos_dpi *ctx)
 {
-	if (ctx->panel)
+	if (ctx->panel) {
+		drm_panel_prepare(ctx->panel);
 		drm_panel_enable(ctx->panel);
+	}
 }
 
 static void exynos_dpi_poweroff(struct exynos_dpi *ctx)
 {
-	if (ctx->panel)
+	if (ctx->panel) {
 		drm_panel_disable(ctx->panel);
+		drm_panel_unprepare(ctx->panel);
+	}
 }
 
 static void exynos_dpi_dpms(struct exynos_drm_display *display, int mode)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 5e78d45..2f58e45 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1333,6 +1333,12 @@  static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	if (ret < 0)
 		return ret;
 
+	ret = drm_panel_prepare(dsi->panel);
+	if (ret < 0) {
+		exynos_dsi_poweroff(dsi);
+		return ret;
+	}
+
 	ret = drm_panel_enable(dsi->panel);
 	if (ret < 0) {
 		exynos_dsi_poweroff(dsi);
@@ -1354,6 +1360,7 @@  static void exynos_dsi_disable(struct exynos_dsi *dsi)
 
 	exynos_dsi_set_display_enable(dsi, false);
 	drm_panel_disable(dsi->panel);
+	drm_panel_unprepare(dsi->panel);
 	exynos_dsi_poweroff(dsi);
 
 	dsi->state &= ~DSIM_STATE_ENABLED;
diff --git a/drivers/gpu/drm/panel/panel-ld9040.c b/drivers/gpu/drm/panel/panel-ld9040.c
index db1601f..046d9fe 100644
--- a/drivers/gpu/drm/panel/panel-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-ld9040.c
@@ -214,7 +214,7 @@  static int ld9040_power_off(struct ld9040 *ctx)
 	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
 }
 
-static int ld9040_disable(struct drm_panel *panel)
+static int ld9040_unprepare(struct drm_panel *panel)
 {
 	struct ld9040 *ctx = panel_to_ld9040(panel);
 
@@ -228,7 +228,12 @@  static int ld9040_disable(struct drm_panel *panel)
 	return ld9040_power_off(ctx);
 }
 
-static int ld9040_enable(struct drm_panel *panel)
+static int ld9040_disable(struct drm_panel *panel)
+{
+	return 0;
+}
+
+static int ld9040_prepare(struct drm_panel *panel)
 {
 	struct ld9040 *ctx = panel_to_ld9040(panel);
 	int ret;
@@ -242,11 +247,16 @@  static int ld9040_enable(struct drm_panel *panel)
 	ret = ld9040_clear_error(ctx);
 
 	if (ret < 0)
-		ld9040_disable(panel);
+		ld9040_unprepare(panel);
 
 	return ret;
 }
 
+static int ld9040_enable(struct drm_panel *panel)
+{
+	return 0;
+}
+
 static int ld9040_get_modes(struct drm_panel *panel)
 {
 	struct drm_connector *connector = panel->connector;
@@ -272,7 +282,9 @@  static int ld9040_get_modes(struct drm_panel *panel)
 }
 
 static const struct drm_panel_funcs ld9040_drm_funcs = {
+	.unprepare = ld9040_unprepare,
 	.disable = ld9040_disable,
+	.prepare = ld9040_prepare,
 	.enable = ld9040_enable,
 	.get_modes = ld9040_get_modes,
 };
diff --git a/drivers/gpu/drm/panel/panel-s6e8aa0.c b/drivers/gpu/drm/panel/panel-s6e8aa0.c
index 06e57a2..51c657a 100644
--- a/drivers/gpu/drm/panel/panel-s6e8aa0.c
+++ b/drivers/gpu/drm/panel/panel-s6e8aa0.c
@@ -887,7 +887,7 @@  static int s6e8aa0_power_off(struct s6e8aa0 *ctx)
 	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
 }
 
-static int s6e8aa0_disable(struct drm_panel *panel)
+static int s6e8aa0_unprepare(struct drm_panel *panel)
 {
 	struct s6e8aa0 *ctx = panel_to_s6e8aa0(panel);
 
@@ -900,7 +900,12 @@  static int s6e8aa0_disable(struct drm_panel *panel)
 	return s6e8aa0_power_off(ctx);
 }
 
-static int s6e8aa0_enable(struct drm_panel *panel)
+static int s6e8aa0_disable(struct drm_panel *panel)
+{
+	return 0;
+}
+
+static int s6e8aa0_prepare(struct drm_panel *panel)
 {
 	struct s6e8aa0 *ctx = panel_to_s6e8aa0(panel);
 	int ret;
@@ -913,11 +918,16 @@  static int s6e8aa0_enable(struct drm_panel *panel)
 	ret = ctx->error;
 
 	if (ret < 0)
-		s6e8aa0_disable(panel);
+		s6e8aa0_unprepare(panel);
 
 	return ret;
 }
 
+static int s6e8aa0_enable(struct drm_panel *panel)
+{
+	return 0;
+}
+
 static int s6e8aa0_get_modes(struct drm_panel *panel)
 {
 	struct drm_connector *connector = panel->connector;
@@ -943,7 +953,9 @@  static int s6e8aa0_get_modes(struct drm_panel *panel)
 }
 
 static const struct drm_panel_funcs s6e8aa0_drm_funcs = {
+	.unprepare = s6e8aa0_unprepare,
 	.disable = s6e8aa0_disable,
+	.prepare = s6e8aa0_prepare,
 	.enable = s6e8aa0_enable,
 	.get_modes = s6e8aa0_get_modes,
 };
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a251361..fb0cfe2 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -45,7 +45,8 @@  struct panel_desc {
 
 struct panel_simple {
 	struct drm_panel base;
-	bool enabled;
+	bool panel_enabled;
+	bool backlight_enabled;
 
 	const struct panel_desc *desc;
 
@@ -93,11 +94,28 @@  static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 	return num;
 }
 
+static int panel_simple_unprepare(struct drm_panel *panel)
+{
+	struct panel_simple *p = to_panel_simple(panel);
+
+	if (!p->panel_enabled)
+		return 0;
+
+	if (p->enable_gpio)
+		gpiod_set_value_cansleep(p->enable_gpio, 0);
+
+	regulator_disable(p->supply);
+
+	p->panel_enabled = false;
+
+	return 0;
+}
+
 static int panel_simple_disable(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
 
-	if (!p->enabled)
+	if (!p->backlight_enabled)
 		return 0;
 
 	if (p->backlight) {
@@ -105,21 +123,17 @@  static int panel_simple_disable(struct drm_panel *panel)
 		backlight_update_status(p->backlight);
 	}
 
-	if (p->enable_gpio)
-		gpiod_set_value_cansleep(p->enable_gpio, 0);
-
-	regulator_disable(p->supply);
-	p->enabled = false;
+	p->backlight_enabled = false;
 
 	return 0;
 }
 
-static int panel_simple_enable(struct drm_panel *panel)
+static int panel_simple_prepare(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
 	int err;
 
-	if (p->enabled)
+	if (p->panel_enabled)
 		return 0;
 
 	err = regulator_enable(p->supply);
@@ -131,12 +145,24 @@  static int panel_simple_enable(struct drm_panel *panel)
 	if (p->enable_gpio)
 		gpiod_set_value_cansleep(p->enable_gpio, 1);
 
+	p->panel_enabled = true;
+
+	return 0;
+}
+
+static int panel_simple_enable(struct drm_panel *panel)
+{
+	struct panel_simple *p = to_panel_simple(panel);
+
+	if (p->backlight_enabled)
+		return 0;
+
 	if (p->backlight) {
 		p->backlight->props.power = FB_BLANK_UNBLANK;
 		backlight_update_status(p->backlight);
 	}
 
-	p->enabled = true;
+	p->backlight_enabled = true;
 
 	return 0;
 }
@@ -164,6 +190,8 @@  static int panel_simple_get_modes(struct drm_panel *panel)
 
 static const struct drm_panel_funcs panel_simple_funcs = {
 	.disable = panel_simple_disable,
+	.unprepare = panel_simple_unprepare,
+	.prepare = panel_simple_prepare,
 	.enable = panel_simple_enable,
 	.get_modes = panel_simple_get_modes,
 };
@@ -178,7 +206,8 @@  static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 	if (!panel)
 		return -ENOMEM;
 
-	panel->enabled = false;
+	panel->panel_enabled = false;
+	panel->backlight_enabled = false;
 	panel->desc = desc;
 
 	panel->supply = devm_regulator_get(dev, "power");
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 446837e..b574ee6 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -139,9 +139,11 @@  static void tegra_encoder_dpms(struct drm_encoder *encoder, int mode)
 
 	if (mode != DRM_MODE_DPMS_ON) {
 		drm_panel_disable(panel);
+		drm_panel_unprepare(panel);
 		tegra_output_disable(output);
 	} else {
 		tegra_output_enable(output);
+		drm_panel_prepare(panel);
 		drm_panel_enable(panel);
 	}
 }