| Message ID | yunobzz1xz8.fsf@aiko.keithp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 08 Aug 2011 12:53:31 -0700 Keith Packard <keithp@keithp.com> wrote: > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> 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.
On Mon, 08 Aug 2011 13:24:12 -0700 Keith Packard <keithp@keithp.com> wrote: > On Mon, 8 Aug 2011 13:01:28 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > On Mon, 08 Aug 2011 12:53:31 -0700 > > Keith Packard <keithp@keithp.com> wrote: > > > > > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> 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@keithp.com> > 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@keithp.com> Yeah looks like a nice improvement. Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 6318828..03a9e6d 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -400,53 +400,17 @@ 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. + /* Safest to just always power off for mode setting. */ - - 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); - } + 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 +1006,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)) {