From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down Date: Mon, 7 Jul 2014 11:30:40 +0200 Message-ID: <20140707093040.GO5821@phenom.ffwll.local> References: <1402655877-6460-1-git-send-email-ville.syrjala@linux.intel.com> <1402655877-6460-12-git-send-email-ville.syrjala@linux.intel.com> <20140625120359.4830a662@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by gabe.freedesktop.org (Postfix) with ESMTP id CAB3089EF7 for ; Mon, 7 Jul 2014 02:30:31 -0700 (PDT) Received: by mail-wi0-f169.google.com with SMTP id hi2so14629730wib.2 for ; Mon, 07 Jul 2014 02:30:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140625120359.4830a662@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Jun 25, 2014 at 12:03:59PM -0700, Jesse Barnes wrote: > On Fri, 13 Jun 2014 13:37:57 +0300 > ville.syrjala@linux.intel.com wrote: > = > > From: Ville Syrj=E4l=E4 > > = > > Set some bits in CCK/CCU to turn off display clocks when disp2d is power > > gated. Not sure this really helps with anything. Docs aren't all that c= lear. > > = > > XXX: Doesn't actually work. CCK_DISPLAY_REF_CLOCK_CONTROL and CCU_ICLK_5 > > writes don't have any effect on the registers for some reason. When clo= ck > > gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force b= it > > gets set but again direct write has no effect. > > --- > > drivers/gpu/drm/i915/i915_reg.h | 5 +++++ > > drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++= ++ > > 2 files changed, 40 insertions(+) > > = > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index 2aa9a3c..be88b13 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -584,12 +584,17 @@ enum punit_power_well { > > #define DSI_PLL_M1_DIV_SHIFT 0 > > #define DSI_PLL_M1_DIV_MASK (0x1ff << 0) > > #define CCK_DISPLAY_CLOCK_CONTROL 0x6b > > +#define CCK_DISPLAY_REF_CLOCK_CONTROL 0x6c > > #define DISPLAY_TRUNK_FORCE_ON (1 << 17) > > #define DISPLAY_TRUNK_FORCE_OFF (1 << 16) > > #define DISPLAY_FREQUENCY_STATUS (0x1f << 8) > > #define DISPLAY_FREQUENCY_STATUS_SHIFT 8 > > #define DISPLAY_FREQUENCY_VALUES (0x1f << 0) > > = > > +#define CCU_ICLK_5 0x114 > > +#define DISPSS_CLKREQ (1 << 1) > > +#define DISPBEND_CLKREQ (1 << 0) > > + > > /** > > * DOC: DPIO > > * > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index d8e20d3..96614c3 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -6057,6 +6057,20 @@ static void vlv_display_power_well_enable(struct= drm_i915_private *dev_priv, > > { > > WARN_ON_ONCE(power_well->data !=3D PUNIT_POWER_WELL_DISP2D); > > = > > + mutex_lock(&dev_priv->dpio_lock); > > + /* > > + * (re)enable ref clocks at CCU > > + * FIXME maybe move to cmnlane? > > + */ > > + vlv_ccu_write(dev_priv, CCU_ICLK_5, > > + vlv_ccu_read(dev_priv, CCU_ICLK_5) | > > + DISPBEND_CLKREQ | DISPSS_CLKREQ); > > + /* > > + * Punit clears CCK trunk force off bits > > + * automagically while ungating disp2d > > + */ > > + mutex_unlock(&dev_priv->dpio_lock); > > + > > vlv_set_power_well(dev_priv, power_well, true); > > = > > spin_lock_irq(&dev_priv->irq_lock); > > @@ -6085,6 +6099,27 @@ static void vlv_display_power_well_disable(struc= t drm_i915_private *dev_priv, > > spin_unlock_irq(&dev_priv->irq_lock); > > = > > vlv_set_power_well(dev_priv, power_well, false); > > + > > + mutex_lock(&dev_priv->dpio_lock); > > + /* > > + * Punit doesn't set the CCK trunk force off bits when power gating > > + * disp2d. It does set them when clock gating disp2d, but we ask it > > + * to power gate instead of clock gate. > > + */ > > + vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, > > + vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) | > > + DISPLAY_TRUNK_FORCE_OFF); > > + vlv_cck_write(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL, > > + vlv_cck_read(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL) | > > + DISPLAY_TRUNK_FORCE_OFF); > > + /* > > + * disable ref clocks at CCU > > + * FIXME maybe move to cmnlane? > > + */ > > + vlv_ccu_write(dev_priv, CCU_ICLK_5, > > + vlv_ccu_read(dev_priv, CCU_ICLK_5) & > > + ~(DISPBEND_CLKREQ | DISPSS_CLKREQ)); > > + mutex_unlock(&dev_priv->dpio_lock); > > } > > = > > static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *de= v_priv, > = > I guess you could ping the hw folks about this, maybe starting with > Cesar, to see if we can drop the power any further by doing this or > poking some other reg... Pulled the entire series except this one here int dinq. Thanks for patches&review. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch