On Thu, Dec 22, 2016 at 01:57:26PM +0200, David Weinehall wrote:

On Thu, Dec 22, 2016 at 01:49:39PM +0200, Ville Syrj�l� wrote:

quoted

On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote:

quoted

On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote:

quoted

From: Ville Syrj�l� <redacted>

Apparently some VLV BIOSen like to leave the VDD force bit enabled
even for power seqeuncers that aren't properly hooked up to any
port. That will result in a imbalance in the AUX power domain
refcount when we stat to use said power sequencer as edp_panel_vdd_on()
will not grab the power domain reference if it sees that the VDD is
already on.

To fix this let's make sure we turn off the VDD force bit when we
initialize the power sequencer registers. That is, unless it's
being done from the init path since there we are actually
initializing the registers for the current power sequencer and
we don't want to turn VDD off needlessly as that would require
waiting for the power cycle delay before we turn it back on.

This fixes the following kind of warnings:
WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]()
WARN_ON(!power_domains->domain_use_count[domain])
...

Cc: stable@vger.kernel.org
Cc: Matwey V. Kornilov <redacted>
Tested-by: Matwey V. Kornilov <redacted>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695
Signed-off-by: Ville Syrj�l� <redacted>

Reviewed-by: David Weinehall <redacted>

Typos and whatnot fixed and patch pushed to dinq. Thanks for the review.

quoted

quoted

quoted

---
 drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 66b5bc80b781..4139122704b3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c

@@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 				    struct intel_dp *intel_dp);
 static void
 intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
-					      struct intel_dp *intel_dp);
+					      struct intel_dp *intel_dp,
+					      bool force_disable_vdd);
 static void
 intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp);
 

@@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
 
 	/* init power sequencer on this pipe and port */
 	intel_dp_init_panel_power_sequencer(dev, intel_dp);
-	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true);
 
 	/*
 	 * Even vdd force doesn't work until we've made

@@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
 	 * Only the HW needs to be reprogrammed, the SW state is fixed and
 	 * has been setup during connector init.
 	 */
-	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false);
 
 	return 0;
 }

@@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
 		      port_name(port), pipe_name(intel_dp->pps_pipe));
 
 	intel_dp_init_panel_power_sequencer(dev, intel_dp);
-	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false);
 }
 
 void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)

@@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp)
 
 	/* init power sequencer on this pipe and port */
 	intel_dp_init_panel_power_sequencer(dev, intel_dp);
-	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true);
 }
 
 static void vlv_pre_enable_dp(struct intel_encoder *encoder,

@@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 
 static void
 intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
-					      struct intel_dp *intel_dp)
+					      struct intel_dp *intel_dp,
+					      bool force_disable_vdd)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 pp_on, pp_off, pp_div, port_sel = 0;

@@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 
 	intel_pps_get_registers(dev_priv, intel_dp, &regs);
 
+	/*
+	 * One some VLV machines the BIOS can leave the VDD

On

quoted

+	 * enabled even on power seqeuencers which aren't
+	 * even hooked up to any port. This would mess up

Remove even here

quoted

+	 * the power domain tracking the first time we pick
+	 * one of these power sequencers for use since
+	 * edp_panel_vdd_on() would notice that the VDD was
+	 * already on and therefore wouldn't even grab the

And here.

quoted

+	 * power domain reference. Disable VDD first to avoid
+	 * this. This also avoids spuriously turning the VDD
+	 * on as soon as the new power seqeuencer gets
+	 * initialized.
+	 */
+	if (force_disable_vdd) {
+		u32 pp = ironlake_get_pp_control(intel_dp);
+
+		WARN(pp & PANEL_POWER_ON, "Panel power already on\n");

Wouldn't this just replace one warning with another?  Or is this
a subset of the other warning?

This should never happen. But I'd rather we get a bug report if it ever
does occur.

Fair enough!

quoted

quoted

quoted

+
+		if (pp & EDP_FORCE_VDD)
+			DRM_DEBUG_KMS("VDD already on, disabling first\n");
+
+		pp &= ~EDP_FORCE_VDD;
+
+		I915_WRITE(regs.pp_ctrl, pp);
+	}
+
 	pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) |
 		(seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT);
 	pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) |

@@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev,
 		vlv_initial_power_sequencer_setup(intel_dp);
 	} else {
 		intel_dp_init_panel_power_sequencer(dev, intel_dp);
-		intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+		intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false);
 	}
 }
 

-- 
2.10.2

Kind regards, David

-- 
Ville Syrj�l�
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrj�l�
Intel OTC