From patchwork Mon Aug 8 20:24:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Packard X-Patchwork-Id: 1046692 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p78KORQt009324 for ; Mon, 8 Aug 2011 20:24:48 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BC4B29F4C0 for ; Mon, 8 Aug 2011 13:24:26 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from keithp.com (home.keithp.com [63.227.221.253]) by gabe.freedesktop.org (Postfix) with ESMTP id 69BFC9E779; Mon, 8 Aug 2011 13:24:16 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by keithp.com (Postfix) with ESMTP id 461E9D74020; Mon, 8 Aug 2011 13:24:16 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at keithp.com Received: from keithp.com ([127.0.0.1]) by localhost (keithp.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id bqqlwU0uMui9; Mon, 8 Aug 2011 13:24:12 -0700 (PDT) Received: by keithp.com (Postfix, from userid 1033) id DC94DD74014; Mon, 8 Aug 2011 13:24:12 -0700 (PDT) Received: from koto.keithp.com (localhost [127.0.0.1]) by keithp.com (Postfix) with ESMTP id D7A32C78007; Mon, 8 Aug 2011 13:24:12 -0700 (PDT) Received: by koto.keithp.com (Postfix, from userid 1488) id 69BD164248D; Mon, 8 Aug 2011 13:24:12 -0700 (PDT) From: Keith Packard To: Jesse Barnes Subject: Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked In-Reply-To: <20110808130128.06654f54@jbarnes-desktop> References: <1312653248-3487-1-git-send-email-keithp@keithp.com> <1312653248-3487-3-git-send-email-keithp@keithp.com> <20110808093010.487c4559@jbarnes-desktop> <20110808114954.23b62dd3@jbarnes-desktop> <20110808130128.06654f54@jbarnes-desktop> User-Agent: Notmuch/0.6.1-66-ga900dda (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu) Date: Mon, 08 Aug 2011 13:24:12 -0700 Message-ID: MIME-Version: 1.0 Cc: Dave Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Mon, 08 Aug 2011 20:24:48 +0000 (UTC) On Mon, 8 Aug 2011 13:01:28 -0700, Jesse Barnes wrote: > On Mon, 08 Aug 2011 12:53:31 -0700 > Keith Packard wrote: > > > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes 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 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 --- 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)) {