diff mbox

[2/6] drm/i915: properly disable the VDD when disabling the panel

Message ID 1394233524-3522-3-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni March 7, 2014, 11:05 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Commit b3064154dfd37deb386b1e459c54e1ca2460b3d5 tried to revert commit
dff392dbd258381a6c3164f38420593f2d291e3b, but wasn't complete, which
resulted in regressions on Haswell. So this commit should fix
b3064154dfd37deb386b1e459c54e1ca2460b3d5 by undoing what it did and
providing an actual complete revert of
dff392dbd258381a6c3164f38420593f2d291e3b.

Fixes regression introduced by:
commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Date:   Tue Mar 4 00:42:44 2014 +0100
    drm/i915: Don't just say it, actually force edp vdd

Testcase: igt/pm_pc8
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 1 +
 drivers/gpu/drm/i915/intel_dp.c  | 9 ++++++---
 drivers/gpu/drm/i915/intel_drv.h | 1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

Comments

Jani Nikula March 14, 2014, 12:07 p.m. UTC | #1 
On Sat, 08 Mar 2014, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Commit b3064154dfd37deb386b1e459c54e1ca2460b3d5 tried to revert commit
> dff392dbd258381a6c3164f38420593f2d291e3b, but wasn't complete, which
> resulted in regressions on Haswell. So this commit should fix
> b3064154dfd37deb386b1e459c54e1ca2460b3d5 by undoing what it did and
> providing an actual complete revert of
> dff392dbd258381a6c3164f38420593f2d291e3b.
>
> Fixes regression introduced by:
> commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
> Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Date:   Tue Mar 4 00:42:44 2014 +0100
>     drm/i915: Don't just say it, actually force edp vdd

Patrik, would you mind acking/testing this patch, as it blames your
commit for regressing?

Thanks,
Jani.


>
> Testcase: igt/pm_pc8
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 1 +
>  drivers/gpu/drm/i915/intel_dp.c  | 9 ++++++---
>  drivers/gpu/drm/i915/intel_drv.h | 1 +
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2665e0..34e8bb3 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1340,6 +1340,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +		edp_panel_vdd_on(intel_dp);
>  		intel_edp_panel_off(intel_dp);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 22e1bdd..e936f36 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -91,7 +91,6 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>  }
>  
>  static void intel_dp_link_down(struct intel_dp *intel_dp);
> -static void edp_panel_vdd_on(struct intel_dp *intel_dp);
>  static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
>  
>  static int
> @@ -1162,7 +1161,7 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
>  	return control;
>  }
>  
> -static void edp_panel_vdd_on(struct intel_dp *intel_dp)
> +void edp_panel_vdd_on(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1338,11 +1337,16 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
>  
>  	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>  
> +	intel_dp->want_panel_vdd = false;
> +
>  	I915_WRITE(pp_ctrl_reg, pp);
>  	POSTING_READ(pp_ctrl_reg);
>  
>  	intel_dp->last_power_cycle = jiffies;
>  	wait_panel_off(intel_dp);
> +
> +	/* We got a reference when we enabled the VDD. */
> +	intel_runtime_pm_put(dev_priv);
>  }
>  
>  void intel_edp_backlight_on(struct intel_dp *intel_dp)
> @@ -1880,7 +1884,6 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>  	intel_edp_backlight_off(intel_dp);
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  	intel_edp_panel_off(intel_dp);
> -	edp_panel_vdd_off(intel_dp, true);
>  
>  	/* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */
>  	if (!(port == PORT_A || IS_VALLEYVIEW(dev)))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c70905..805d207 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -762,6 +762,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp);
>  void intel_edp_psr_enable(struct intel_dp *intel_dp);
>  void intel_edp_psr_disable(struct intel_dp *intel_dp);
>  void intel_edp_psr_update(struct drm_device *dev);
> +void edp_panel_vdd_on(struct intel_dp *intel_dp);
>  
>  
>  /* intel_dsi.c */
> -- 
> 1.8.5.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter March 14, 2014, 1:57 p.m. UTC | #2 
On Fri, Mar 14, 2014 at 1:07 PM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
>> Fixes regression introduced by:
>> commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
>> Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>> Date:   Tue Mar 4 00:42:44 2014 +0100
>>     drm/i915: Don't just say it, actually force edp vdd
>
> Patrik, would you mind acking/testing this patch, as it blames your
> commit for regressing?

Testing will be hard I guess since it does the equivalent change
Patrik has done to the pre-hsw dp code to the ddi hsw+ code. But a
review would be nice - I've signed up Damien for this but he seems to
be sick :(
-Daniel
Patrik Jakobsson March 14, 2014, 7 p.m. UTC | #3 
On Fri, Mar 14, 2014 at 2:57 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Mar 14, 2014 at 1:07 PM, Jani Nikula
> <jani.nikula@linux.intel.com> wrote:
>>> Fixes regression introduced by:
>>> commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
>>> Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>>> Date:   Tue Mar 4 00:42:44 2014 +0100
>>>     drm/i915: Don't just say it, actually force edp vdd
>>
>> Patrik, would you mind acking/testing this patch, as it blames your
>> commit for regressing?
>
> Testing will be hard I guess since it does the equivalent change
> Patrik has done to the pre-hsw dp code to the ddi hsw+ code. But a
> review would be nice - I've signed up Damien for this but he seems to
> be sick :(
> -Daniel

Yes, as expected, this patch applied on top of -nightly is working fine. It's
the EDP_FORCE_VDD bit that is required for my Macbook Air 6,x.

Is the proper fix for -fixes to completely revert:
dff392dbd258381a6c3164f38420593f2d291e3b

Tested-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

Thanks
Patrik
Jani Nikula March 18, 2014, 11:27 a.m. UTC | #4 
On Fri, 14 Mar 2014, Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote:
> On Fri, Mar 14, 2014 at 2:57 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Fri, Mar 14, 2014 at 1:07 PM, Jani Nikula
>> <jani.nikula@linux.intel.com> wrote:
>>>> Fixes regression introduced by:
>>>> commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
>>>> Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>>>> Date:   Tue Mar 4 00:42:44 2014 +0100
>>>>     drm/i915: Don't just say it, actually force edp vdd
>>>
>>> Patrik, would you mind acking/testing this patch, as it blames your
>>> commit for regressing?
>>
>> Testing will be hard I guess since it does the equivalent change
>> Patrik has done to the pre-hsw dp code to the ddi hsw+ code. But a
>> review would be nice - I've signed up Damien for this but he seems to
>> be sick :(
>> -Daniel
>
> Yes, as expected, this patch applied on top of -nightly is working fine. It's
> the EDP_FORCE_VDD bit that is required for my Macbook Air 6,x.
>
> Is the proper fix for -fixes to completely revert:
> dff392dbd258381a6c3164f38420593f2d291e3b

Hi Patrik, I've done just that. Giving the drm-intel-fixes branch a spin
would be great, if you don't mind me asking.

BR,
Jani.

>
> Tested-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>
> Thanks
> Patrik
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2665e0..34e8bb3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1340,6 +1340,7 @@  static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+		edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 22e1bdd..e936f36 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -91,7 +91,6 @@  static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 }
 
 static void intel_dp_link_down(struct intel_dp *intel_dp);
-static void edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
 
 static int
@@ -1162,7 +1161,7 @@  static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
 	return control;
 }
 
-static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+void edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1338,11 +1337,16 @@  void intel_edp_panel_off(struct intel_dp *intel_dp)
 
 	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
 
+	intel_dp->want_panel_vdd = false;
+
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
 	intel_dp->last_power_cycle = jiffies;
 	wait_panel_off(intel_dp);
+
+	/* We got a reference when we enabled the VDD. */
+	intel_runtime_pm_put(dev_priv);
 }
 
 void intel_edp_backlight_on(struct intel_dp *intel_dp)
@@ -1880,7 +1884,6 @@  static void intel_disable_dp(struct intel_encoder *encoder)
 	intel_edp_backlight_off(intel_dp);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 	intel_edp_panel_off(intel_dp);
-	edp_panel_vdd_off(intel_dp, true);
 
 	/* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */
 	if (!(port == PORT_A || IS_VALLEYVIEW(dev)))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9c70905..805d207 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -762,6 +762,7 @@  void intel_edp_panel_off(struct intel_dp *intel_dp);
 void intel_edp_psr_enable(struct intel_dp *intel_dp);
 void intel_edp_psr_disable(struct intel_dp *intel_dp);
 void intel_edp_psr_update(struct drm_device *dev);
+void edp_panel_vdd_on(struct intel_dp *intel_dp);
 
 
 /* intel_dsi.c */