Re: [PATCH 10/24] drm/i915: enable power wells on haswell init

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 26 Apr 2012 15:21:05 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> This attempts to enable all the available power wells during the
> initialization.
> 
> Those power wells can be enabled in parallel or on-demand, and disabled
> when no longer needed, but this is out of scope of this initial
> enablement. Proper tracking of who uses which power well will require
> a considerable rework of our display handling, so we just leave them all
> enabled when the driver is loaded for now.
> 
> v2: use more generic and future-proof code
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |   36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 06f38ec..f87768d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2937,6 +2937,37 @@ void intel_init_clock_gating(struct drm_device *dev)
>  		dev_priv->display.init_pch_clock_gating(dev);
>  }
>  
> +/* Starting with Haswell, we have different power wells for
> + * different parts of the GPU. This attempts to enable them all.
> + */
> +void intel_init_power_wells(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned long power_wells[] = {
> +		HSW_PWR_WELL_CTL1,
> +		HSW_PWR_WELL_CTL2,
> +		HSW_PWR_WELL_CTL4
> +	};
> +	int i;
> +
> +	if (!IS_HASWELL(dev))
> +		return;
> +
> +	mutex_lock(&dev->struct_mutex);
> +
> +	for (i = 0; i < ARRAY_SIZE(power_wells); i++) {
> +		int well = I915_READ(power_wells[i]);
> +
> +		if ((well & HSW_PWR_WELL_STATE) == 0) {
> +			I915_WRITE(power_wells[i], well & HSW_PWR_WELL_ENABLE);
> +			if (wait_for(I915_READ(power_wells[i] & HSW_PWR_WELL_STATE), 20))
> +				DRM_ERROR("Error enabling power well %lx\n", power_wells[i]);
> +		}
> +	}
> +
> +	mutex_unlock(&dev->struct_mutex);
> +}
> +
>  /* Set up chip specific power management-related functions */
>  void intel_init_pm(struct drm_device *dev)
>  {
> @@ -3077,5 +3108,10 @@ void intel_init_pm(struct drm_device *dev)
>  		else
>  			dev_priv->display.get_fifo_size = i830_get_fifo_size;
>  	}
> +
> +	/* We attempt to init the necessary power wells early in the initialization
> +	 * time, so the subsystems that expect power to be enabled can work.
> +	 */
> +	intel_init_power_wells(dev);
>  }
>  

Note that many of the regs are controls like we have in the IVB MT
forcewake path rather than separate power planes in the GPU (i.e. one
of many agents can wake up the GPU for whatever reason: BIOS, driver,
KVMr, etc).  At least that's what it looks like to me, maybe you have
more up-to-date info.

That said, it's important for us to clear the driver and debug power
well bits when we shut down the display since it should save us a lot of
power.

-- 
Jesse Barnes, Intel Open Source Technology Center