diff mbox

[2/4] drm/i915: Leave LVDS registers unlocked

Message ID 1312653248-3487-3-git-send-email-keithp@keithp.com
State New, archived
Headers show

Commit Message

Keith Packard Aug. 6, 2011, 5:54 p.m. UTC
There's no reason to relock them; it just makes operations more
complex. This fixes DPMS where the panel registers were locked making
the disable not work.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   46 +++++++++++++++---------------------
 1 files changed, 19 insertions(+), 27 deletions(-)

Comments

Jesse Barnes Aug. 8, 2011, 4:30 p.m. UTC | #1
On Sat,  6 Aug 2011 10:54:06 -0700
Keith Packard <keithp@keithp.com> wrote:

> There's no reason to relock them; it just makes operations more
> complex. This fixes DPMS where the panel registers were locked making
> the disable not work.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

Yep, looks fine.  The only think we might want to sprinkle about are
checks for panel off so we can avoid visible corruption if we whack
timing or fb stuff while the panel is on.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Keith Packard Aug. 8, 2011, 6:25 p.m. UTC | #2
On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yep, looks fine.  The only think we might want to sprinkle about are
> checks for panel off so we can avoid visible corruption if we whack
> timing or fb stuff while the panel is on.

Yeah, could do. Would be nice to somehow get the LVDS code ripped out of
the middle of intel_display.c; everything in intel_lvds.c is nicely
bracketed by prepare/commit, which turn the panel off and back on.
Keith Packard Aug. 8, 2011, 6:42 p.m. UTC | #3
On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yep, looks fine.  The only think we might want to sprinkle about are
> checks for panel off so we can avoid visible corruption if we whack
> timing or fb stuff while the panel is on.

So, I'd like to know if we could unlock the panel registers on pre-PCH
hardware as well at init time; that way I could remove the unlock code
From intel_lvds_prepare too. Should it be possible to set the
PANEL_UNLOCK_REGS bits for pre-PCH hardware without having the target off?
Jesse Barnes Aug. 8, 2011, 6:49 p.m. UTC | #4
On Mon, 08 Aug 2011 11:42:56 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Yep, looks fine.  The only think we might want to sprinkle about are
> > checks for panel off so we can avoid visible corruption if we whack
> > timing or fb stuff while the panel is on.
> 
> So, I'd like to know if we could unlock the panel registers on pre-PCH
> hardware as well at init time; that way I could remove the unlock code
> From intel_lvds_prepare too. Should it be possible to set the
> PANEL_UNLOCK_REGS bits for pre-PCH hardware without having the target off?

Yep, it's safe and possible to do on pre-PCH as well.  For panel
fitting we do need to do an actual power cycle when going from
non-native back to native iirc, but we can still leave them unlocked so
we don't have to worry about the lock/unlock sequence everywhere.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6985e42..cc85618 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -414,46 +414,32 @@  static void intel_lvds_prepare(struct drm_encoder *encoder)
 	/* We try to do the minimum that is necessary in order to unlock
 	 * the registers for mode setting.
 	 *
-	 * On Ironlake, this is quite simple as we just set the unlock key
-	 * and ignore all subtleties. (This may cause some issues...)
+	 * On Ironlake, this is quite simple as we just set the unlock
+	 * key in the driver init code and ignore all subtleties.
+	 * (This may cause some issues...)
 	 *
 	 * Prior to Ironlake, we must disable the pipe if we want to adjust
 	 * the panel fitter. However at all other times we can just reset
 	 * the registers regardless.
 	 */
 
-	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(PCH_PP_CONTROL,
-			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
-	} else if (intel_lvds->pfit_dirty) {
-		I915_WRITE(PP_CONTROL,
-			   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
-			   & ~POWER_TARGET_ON);
-	} else {
-		I915_WRITE(PP_CONTROL,
-			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
+	if (!HAS_PCH_SPLIT(dev)) {
+		if (intel_lvds->pfit_dirty) {
+			I915_WRITE(PP_CONTROL,
+				   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
+				   & ~POWER_TARGET_ON);
+		} else {
+			I915_WRITE(PP_CONTROL,
+				   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
+		}
 	}
+	intel_lvds_disable(intel_lvds);
 }
 
 static void intel_lvds_commit(struct drm_encoder *encoder)
 {
-	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/* Undo any unlocking done in prepare to prevent accidental
-	 * adjustment of the registers.
-	 */
-	if (HAS_PCH_SPLIT(dev)) {
-		u32 val = I915_READ(PCH_PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PCH_PP_CONTROL, val & 0x3);
-	} else {
-		u32 val = I915_READ(PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PP_CONTROL, val & 0x3);
-	}
-
 	/* Always do a full power on as we do not know what state
 	 * we were left in.
 	 */
@@ -1049,6 +1035,12 @@  out:
 		pwm = I915_READ(BLC_PWM_PCH_CTL1);
 		pwm |= PWM_PCH_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PCH_PP_CONTROL,
+			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
 	}
 	dev_priv->lid_notifier.notifier_call = intel_lid_notify;
 	if (acpi_lid_notifier_register(&dev_priv->lid_notifier)) {