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

Jesse Barnes
2014-06-25T19:03:25Z

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...

--
Jesse Barnes, Intel Open Source Technology Center