Re: [WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down

[Daniel Vetter <daniel@ffwll.ch>]

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älä <ville.syrjala@linux.intel.com>
> > 
> > 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 clear.
> > 
> > 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 clock
> > gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force bit
> > 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/i915_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/intel_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 != 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(struct 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 *dev_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