From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/9] drm/i915: Skip modifying PCH DREF if not changing clock sources Date: Tue, 2 Apr 2013 19:53:57 +0200 Message-ID: <20130402175357.GV2228@phenom.ffwll.local> References: <1364340792-7278-1-git-send-email-jbarnes@virtuousgeek.org> <1364340792-7278-2-git-send-email-jbarnes@virtuousgeek.org> <87vc8cnfvg.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f176.google.com (mail-ea0-f176.google.com [209.85.215.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 3E2C4E5C30 for ; Tue, 2 Apr 2013 10:51:02 -0700 (PDT) Received: by mail-ea0-f176.google.com with SMTP id h10so354910eaj.35 for ; Tue, 02 Apr 2013 10:51:01 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87vc8cnfvg.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Mar 28, 2013 at 09:27:31AM +0200, Jani Nikula wrote: > On Wed, 27 Mar 2013, Jesse Barnes wrote: > > From: Chris Wilson > > > > Modifying the clock sources (via the DREF control on the PCH) is a slow > > multi-stage process as we need to let the clocks stabilise between each > > stage. If we are not actually changing the clock sources, then we can > > return early. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++--------- > > 1 file changed, 61 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 8f0db8c..9d05b30 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4833,7 +4833,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct drm_mode_config *mode_config = &dev->mode_config; > > struct intel_encoder *encoder; > > - u32 temp; > > + u32 val, final; > > bool has_lvds = false; > > bool has_cpu_edp = false; > > bool has_pch_edp = false; > > @@ -4876,70 +4876,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > > * PCH B stepping, previous chipset stepping should be > > * ignoring this setting. > > */ > > - temp = I915_READ(PCH_DREF_CONTROL); > > + val = I915_READ(PCH_DREF_CONTROL); > > + > > + /* As we must carefully and slowly disable/enable each source in turn, > > + * compute the final state we want first and check if we need to > > + * make any changes at all. > > + */ > > + final = val; > > + final &= ~DREF_NONSPREAD_SOURCE_MASK; > > + if (has_ck505) > > + final |= DREF_NONSPREAD_CK505_ENABLE; > > + else > > + final |= DREF_NONSPREAD_SOURCE_ENABLE; > > + > > + final &= ~DREF_SSC_SOURCE_MASK; > > + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > + final &= ~DREF_SSC1_ENABLE; > > + > > + if (has_panel) { > > + final |= DREF_SSC_SOURCE_ENABLE; > > + > > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > > + final |= DREF_SSC1_ENABLE; > > + > > + if (has_cpu_edp) { > > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > > + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > > + else > > + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > > + } else > > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > I've been assimilated, I dislike not having braces in all branches if > one branch requires them... but that's bikeshedding. On the stuff that > matters, I've punted on this bikeshed (checkpatch didn't yell), but fixed another one ;-) > > Reviewed-by: Jani Nikula I've figured that speeding this up and moving it into ->global_modeset_resources are rather orthogonal, since even with fastboot we might want to adjust pm state a bit (e.g. with the power wells stuff). So even when this is move into the modeset code and thought more clever tricks, we'll still run it in the boot-up modeset path. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch