All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: enable vdd when switching off the eDP panel
@ 2012-05-20 15:14 Daniel Vetter
  2012-05-21 16:40 ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2012-05-20 15:14 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, stable

We have one bug report from a validation team that we get the eDP
panel sequencing still somewhat wrong: We need to enable VDD while
switching off the panel and backlight. Unfortunately that reporter
seems to have fallen off the earth :(

For another reporter this actually fixes a black panel issue because
without this the backlight/panel gets confused and doesn't light up
again.

v2: I've forgotten to remove the vdd_off call in panel_off which is
now bogus. This essentially reverts

commit 17038de5f16569a25343cf68668f3b657eafb00e
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Apr 16 22:43:42 2012 +0100

    drm/i915/dp: Flush any outstanding work to turn the VDD off

v3: the current panel_off code forces off the vdd power, too. Which is
bogus and resulted in some funny warnings later on when we've tried to
do aux channel communications with just the vdd forced on. Fix this,
too.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46312
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43163
Tested-by: Vincent Frentzel <zcecc22@gmail.com>
Cc: stable@kernel.org
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a1a5ce7..3bbd754 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1154,11 +1154,10 @@ static void ironlake_edp_panel_off(struct intel_dp *intel_dp)
 
 	DRM_DEBUG_KMS("Turn eDP power off\n");
 
-	WARN(intel_dp->want_panel_vdd, "Cannot turn power off while VDD is on\n");
-	ironlake_panel_vdd_off_sync(intel_dp); /* finish any pending work */
+	WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
 
 	pp = ironlake_get_pp_control(dev_priv);
-	pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | EDP_BLC_ENABLE);
+	pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);
 	I915_WRITE(PCH_PP_CONTROL, pp);
 	POSTING_READ(PCH_PP_CONTROL);
 
@@ -1266,18 +1265,16 @@ static void intel_dp_prepare(struct drm_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+
+	/* Make sure the panel is off before trying to change the mode. But also
+	 * ensure that we have vdd while we switch off the panel. */
+	ironlake_edp_panel_vdd_on(intel_dp);
 	ironlake_edp_backlight_off(intel_dp);
 	ironlake_edp_panel_off(intel_dp);
 
-	/* Wake up the sink first */
-	ironlake_edp_panel_vdd_on(intel_dp);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
 	intel_dp_link_down(intel_dp);
 	ironlake_edp_panel_vdd_off(intel_dp, false);
-
-	/* Make sure the panel is off before trying to
-	 * change the mode
-	 */
 }
 
 static void intel_dp_commit(struct drm_encoder *encoder)
@@ -1309,10 +1306,11 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 	uint32_t dp_reg = I915_READ(intel_dp->output_reg);
 
 	if (mode != DRM_MODE_DPMS_ON) {
+		/* Switching the panel off requires vdd. */
+		ironlake_edp_panel_vdd_on(intel_dp);
 		ironlake_edp_backlight_off(intel_dp);
 		ironlake_edp_panel_off(intel_dp);
 
-		ironlake_edp_panel_vdd_on(intel_dp);
 		intel_dp_sink_dpms(intel_dp, mode);
 		intel_dp_link_down(intel_dp);
 		ironlake_edp_panel_vdd_off(intel_dp, false);
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: enable vdd when switching off the eDP panel
  2012-05-20 15:14 [PATCH] drm/i915: enable vdd when switching off the eDP panel Daniel Vetter
@ 2012-05-21 16:40 ` Daniel Vetter
  2012-05-21 19:09   ` Chris Wilson
  2012-05-22  1:49   ` Keith Packard
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Vetter @ 2012-05-21 16:40 UTC (permalink / raw)
  To: Intel Graphics Development, Keith Packard, Chris Wilson
  Cc: Daniel Vetter, stable

Hi Keith&Chris,

This is the patch for you to try on the macbook airs ....

Thanks, Daniel

On Sun, May 20, 2012 at 05:14:50PM +0200, Daniel Vetter wrote:
> We have one bug report from a validation team that we get the eDP
> panel sequencing still somewhat wrong: We need to enable VDD while
> switching off the panel and backlight. Unfortunately that reporter
> seems to have fallen off the earth :(
> 
> For another reporter this actually fixes a black panel issue because
> without this the backlight/panel gets confused and doesn't light up
> again.
> 
> v2: I've forgotten to remove the vdd_off call in panel_off which is
> now bogus. This essentially reverts
> 
> commit 17038de5f16569a25343cf68668f3b657eafb00e
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Mon Apr 16 22:43:42 2012 +0100
> 
>     drm/i915/dp: Flush any outstanding work to turn the VDD off
> 
> v3: the current panel_off code forces off the vdd power, too. Which is
> bogus and resulted in some funny warnings later on when we've tried to
> do aux channel communications with just the vdd forced on. Fix this,
> too.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46312
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43163
> Tested-by: Vincent Frentzel <zcecc22@gmail.com>
> Cc: stable@kernel.org
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a1a5ce7..3bbd754 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1154,11 +1154,10 @@ static void ironlake_edp_panel_off(struct intel_dp *intel_dp)
>  
>  	DRM_DEBUG_KMS("Turn eDP power off\n");
>  
> -	WARN(intel_dp->want_panel_vdd, "Cannot turn power off while VDD is on\n");
> -	ironlake_panel_vdd_off_sync(intel_dp); /* finish any pending work */
> +	WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
>  
>  	pp = ironlake_get_pp_control(dev_priv);
> -	pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | EDP_BLC_ENABLE);
> +	pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);
>  	I915_WRITE(PCH_PP_CONTROL, pp);
>  	POSTING_READ(PCH_PP_CONTROL);
>  
> @@ -1266,18 +1265,16 @@ static void intel_dp_prepare(struct drm_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  
> +
> +	/* Make sure the panel is off before trying to change the mode. But also
> +	 * ensure that we have vdd while we switch off the panel. */
> +	ironlake_edp_panel_vdd_on(intel_dp);
>  	ironlake_edp_backlight_off(intel_dp);
>  	ironlake_edp_panel_off(intel_dp);
>  
> -	/* Wake up the sink first */
> -	ironlake_edp_panel_vdd_on(intel_dp);
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>  	intel_dp_link_down(intel_dp);
>  	ironlake_edp_panel_vdd_off(intel_dp, false);
> -
> -	/* Make sure the panel is off before trying to
> -	 * change the mode
> -	 */
>  }
>  
>  static void intel_dp_commit(struct drm_encoder *encoder)
> @@ -1309,10 +1306,11 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
>  	uint32_t dp_reg = I915_READ(intel_dp->output_reg);
>  
>  	if (mode != DRM_MODE_DPMS_ON) {
> +		/* Switching the panel off requires vdd. */
> +		ironlake_edp_panel_vdd_on(intel_dp);
>  		ironlake_edp_backlight_off(intel_dp);
>  		ironlake_edp_panel_off(intel_dp);
>  
> -		ironlake_edp_panel_vdd_on(intel_dp);
>  		intel_dp_sink_dpms(intel_dp, mode);
>  		intel_dp_link_down(intel_dp);
>  		ironlake_edp_panel_vdd_off(intel_dp, false);
> -- 
> 1.7.10
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: enable vdd when switching off the eDP panel
  2012-05-21 16:40 ` Daniel Vetter
@ 2012-05-21 19:09   ` Chris Wilson
  2012-05-22  1:49   ` Keith Packard
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2012-05-21 19:09 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development, Keith Packard
  Cc: Daniel Vetter, stable

On Mon, 21 May 2012 18:40:30 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> Hi Keith&Chris,
> 
> This is the patch for you to try on the macbook airs ....

Cursory testing of switching on and off the internal panel seems ok.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: enable vdd when switching off the eDP panel
  2012-05-21 16:40 ` Daniel Vetter
  2012-05-21 19:09   ` Chris Wilson
@ 2012-05-22  1:49   ` Keith Packard
  2012-05-22  6:14     ` Daniel Vetter
  1 sibling, 1 reply; 5+ messages in thread
From: Keith Packard @ 2012-05-22  1:49 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development, Chris Wilson
  Cc: Daniel Vetter, stable


[-- Attachment #1.1: Type: text/plain, Size: 802 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1154,11 +1154,10 @@ static void ironlake_edp_panel_off(struct intel_dp *intel_dp)
>>  
>>  	DRM_DEBUG_KMS("Turn eDP power off\n");
>>  
>> -	WARN(intel_dp->want_panel_vdd, "Cannot turn power off while VDD is on\n");
>> -	ironlake_panel_vdd_off_sync(intel_dp); /* finish any pending work */
>> +	WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
>>  
>>  	pp = ironlake_get_pp_control(dev_priv);
>> -	pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | EDP_BLC_ENABLE);
>> +	pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);

Can you explain where the panel is getting turned off here?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: enable vdd when switching off the eDP panel
  2012-05-22  1:49   ` Keith Packard
@ 2012-05-22  6:14     ` Daniel Vetter
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2012-05-22  6:14 UTC (permalink / raw)
  To: Keith Packard; +Cc: Daniel Vetter, Intel Graphics Development, stable

On Mon, May 21, 2012 at 06:49:18PM -0700, Keith Packard wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1154,11 +1154,10 @@ static void ironlake_edp_panel_off(struct intel_dp *intel_dp)
> >>  
> >>  	DRM_DEBUG_KMS("Turn eDP power off\n");
> >>  
> >> -	WARN(intel_dp->want_panel_vdd, "Cannot turn power off while VDD is on\n");
> >> -	ironlake_panel_vdd_off_sync(intel_dp); /* finish any pending work */
> >> +	WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
> >>  
> >>  	pp = ironlake_get_pp_control(dev_priv);
> >> -	pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | EDP_BLC_ENABLE);
> >> +	pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);
> 
> Can you explain where the panel is getting turned off here?

Well, before this change the edp power off sequence was:
- switch vdd off (or more precise: ensure it's off)
- switch backlight off
- switch panel power off (i.e. the above function)

Now the new sequence is (always with the right amounts of delays,
obviously):
- switch vdd on
- switch backlight off
- switch panel off
- switch vdd off

That's why I've had to remove the EDP_FORCE_VDD bit from the above
bitfrobbing, because we now want vdd to stay on while we do the power
sequence above.

The reason is that we have a callchain (in intel_dp_prepare) where we do
some aux channel communication after having switched off the panel. And
that needs vdd, but because edp_panel_off killed that, things got
confused.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-05-22  6:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-20 15:14 [PATCH] drm/i915: enable vdd when switching off the eDP panel Daniel Vetter
2012-05-21 16:40 ` Daniel Vetter
2012-05-21 19:09   ` Chris Wilson
2012-05-22  1:49   ` Keith Packard
2012-05-22  6:14     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.