Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

On Mon, 8 Aug 2011 13:01:28 -0700, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote:
> On Mon, 08 Aug 2011 12:53:31 -0700
> Keith Packard <keithp@xxxxxxxxxx> wrote:
> 
> > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote:
> > 
> > > 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.
> > 
> > Hidden in the unlock patch was a call to intel_lvds_disable from
> > intel_lvds_prepare -- that *always* turns off the LVDS for mode
> > setting. Do we care enough about LVDS mode setting performance that we
> > should try leave the optimization in place that doesn't turn off the
> > backlight when switching between modes?
> 
> We hate flicker right?  But generally yes it's safer to just turn it
> off all the time.

I'll leave the optimization in place then; it's been there for a while
so at least it shouldn't cause any regressions.

How about this? Has the advantage of not lying in the commit message
anymore.

From 092719152aa5a235d6678798a34dc784d5cec2ad Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@xxxxxxxxxx>
Date: Sat, 6 Aug 2011 10:33:12 -0700
Subject: [PATCH 2/5] drm/i915: Leave LVDS registers unlocked

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@xxxxxxxxxx>
---
 drivers/gpu/drm/i915/intel_lvds.c |   51 +++++++++++-------------------------
 1 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6318828..8b521a2 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -400,53 +400,21 @@ out:
 
 static void intel_lvds_prepare(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);
 
-	/* 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...)
-	 *
+	/*
 	 * 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(encoder->dev) && intel_lvds->pfit_dirty)
+		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.
 	 */
@@ -1042,6 +1010,19 @@ 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);
+	} else {
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PP_CONTROL,
+			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
 	}
 	dev_priv->lid_notifier.notifier_call = intel_lid_notify;
 	if (acpi_lid_notifier_register(&dev_priv->lid_notifier)) {
-- 
1.7.5.4



-- 
keith.packard@xxxxxxxxx

Attachment: pgp00000.pgp
Description: PGP signature