On Fri, 18 Jan 2013, Paulo Zanoni <przanoni at gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> The current code was wrong in many different ways, so this is a full
> rewrite. We don't have "different power wells for different parts of
> the GPU", we have a single power well, but we have multiple registers
> that can be used to request enabling/disabling the power well. So
> let's be a good citizen and only use the register we're supposed to
> use, except when we're loading the driver, where we clear the request
> made by the BIOS.
>
> If any of the registers is requesting the power well to be enabled, it
> will be enabled. If none of the registers is requesting the power well
> to be enabled, it will be disabled.
>
> For now we're just forcing the power well to be enabled, but in the
> next commits we'll change this.
Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 8 ++--
> drivers/gpu/drm/i915/intel_display.c | 5 +--
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 70 +++++++++++++++++++++++++---------
> 4 files changed, 59 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2521617..f054554 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4414,10 +4414,10 @@
> #define AUDIO_CP_READY_C (1<<9)
>
> /* HSW Power Wells */
> -#define HSW_PWR_WELL_CTL1 0x45400 /* BIOS */
> -#define HSW_PWR_WELL_CTL2 0x45404 /* Driver */
> -#define HSW_PWR_WELL_CTL3 0x45408 /* KVMR */
> -#define HSW_PWR_WELL_CTL4 0x4540C /* Debug */
> +#define HSW_PWR_WELL_BIOS 0x45400 /* CTL1 */
> +#define HSW_PWR_WELL_DRIVER 0x45404 /* CTL2 */
> +#define HSW_PWR_WELL_KVMR 0x45408 /* CTL3 */
> +#define HSW_PWR_WELL_DEBUG 0x4540C /* CTL4 */
> #define HSW_PWR_WELL_ENABLE (1<<31)
> #define HSW_PWR_WELL_STATE (1<<30)
> #define HSW_PWR_WELL_CTL5 0x45410
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b35902e..4a9f048 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8647,10 +8647,7 @@ static void i915_disable_vga(struct drm_device *dev)
>
> void intel_modeset_init_hw(struct drm_device *dev)
> {
> - /* 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);
> + intel_init_power_well(dev);
>
> intel_prepare_ddi(dev);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index aeff0d1..8cfad75 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -666,7 +666,7 @@ extern void intel_update_fbc(struct drm_device *dev);
> extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
> extern void intel_gpu_ips_teardown(void);
>
> -extern void intel_init_power_wells(struct drm_device *dev);
> +extern void intel_init_power_well(struct drm_device *dev);
> extern void intel_enable_gt_powersave(struct drm_device *dev);
> extern void intel_disable_gt_powersave(struct drm_device *dev);
> extern void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5a8a72c..2273b9c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4043,33 +4043,69 @@ void intel_init_clock_gating(struct drm_device *dev)
> dev_priv->display.init_clock_gating(dev);
> }
>
> -/* Starting with Haswell, we have different power wells for
> - * different parts of the GPU. This attempts to enable them all.
> +static void intel_set_power_well(struct drm_device *dev, bool enable)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + bool is_enabled, enable_requested;
> + uint32_t tmp;
> +
> + tmp = I915_READ(HSW_PWR_WELL_DRIVER);
> + is_enabled = !!(tmp & HSW_PWR_WELL_STATE);
> + enable_requested = !!(tmp & HSW_PWR_WELL_ENABLE);
> +
> + if (enable) {
> + if (!enable_requested)
> + I915_WRITE(HSW_PWR_WELL_DRIVER, HSW_PWR_WELL_ENABLE);
> +
> + if (!is_enabled) {
> + DRM_DEBUG_KMS("Enabling power well\n");
> + if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> + HSW_PWR_WELL_STATE), 20))
> + DRM_ERROR("Timeout enabling power well\n");
> + }
> + } else {
> + if (enable_requested)
> + I915_WRITE(HSW_PWR_WELL_DRIVER, 0);
> +
> + if (is_enabled) {
> + if (I915_READ(HSW_PWR_WELL_BIOS) & HSW_PWR_WELL_ENABLE)
> + DRM_DEBUG_KMS("Not disabling power well: requested by BIOS\n");
> + else if (I915_READ(HSW_PWR_WELL_KVMR) & HSW_PWR_WELL_ENABLE)
> + DRM_DEBUG_KMS("Not disabling power well: requested by KVMR\n");
> + else if (I915_READ(HSW_PWR_WELL_DEBUG) & HSW_PWR_WELL_ENABLE)
> + DRM_DEBUG_KMS("Not disabling power well: requested by DEBUG\n");
> + else {
> + DRM_DEBUG_KMS("Disabling power well\n");
> + if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> + HSW_PWR_WELL_STATE) == 0, 20))
> + DRM_ERROR("Timeout disabling power well\n");
> + }
> + }
> + }
> +}
> +
> +/*
> + * Starting with Haswell, we have a "Power Down Well" that can be turned off
> + * when not needed anymore. We have 4 registers that can request the power well
> + * to be enabled, and it will only be disabled if none of the registers is
> + * requesting it to be enabled.
> */
> -void intel_init_power_wells(struct drm_device *dev)
> +void intel_init_power_well(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]);
> + /* For now, we need the power well to be always enabled. */
> + intel_set_power_well(dev, true);
>
> - 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]);
> - }
> - }
> + /* We're taking over the BIOS, so clear any requests made by it since
> + * the driver is in charge now. */
> + if (I915_READ(HSW_PWR_WELL_BIOS) & HSW_PWR_WELL_ENABLE)
> + I915_WRITE(HSW_PWR_WELL_BIOS, 0);
>
> mutex_unlock(&dev->struct_mutex);
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
