Attachment 128576 Details for Bug 98695 – [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it

[patch] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it

0001-drm-i915-Force-VDD-off-on-the-new-power-seqeuencer-b.patch (text/plain), 5.51 KB, created by Ville Syrjala on 2016-12-20 11:02:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ville Syrjala

Created: 2016-12-20 11:02:57 UTC

Size: 5.51 KB

patch

obsolete

>From 5132f3b37819020ad0008784e4e1a76acea9d67c Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
>Date: Tue, 20 Dec 2016 12:50:56 +0200
>Subject: [PATCH] drm/i915: Force VDD off on the new power seqeuencer before
> starting to use it
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>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 <matwey.kornilov@gmail.com>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695
>Signed-off-by: Ville SyrjÃ¤lÃ¤ <ville.syrjala@linux.intel.com>
>---
> 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
>+	 * enabled even on power seqeuencers which aren't
>+	 * even hooked up to any port. This would mess up
>+	 * 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
>+	 * 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");
>+
>+		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
>

Actions: View | Splinter Review

Attachments on bug 98695: 127931 | 127932 | 128561 | 128568 | 128576 | 128593

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.