Re: [PATCH 3/4] drm/i915: get a PC8 reference when enabling the power well

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2523 bytes --]

On Wed, 2013-10-30 at 19:40 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> In the current code, at haswell_modeset_global_resources, first we
> decide if we want to enable/disable the power well, then we decide if
> we want to enable/disable PC8. On the case where we're enabling PC8
> this works fine, but on the case where we disable PC8 due to a non-eDP
> monitor being enabled, we first enable the power well and then disable
> PC8. Although wrong, this doesn't seem to be causing any problems now,
> and we don't even see anything in dmesg. But the patches for runtime
> D3 turn this problem into a real bug, so we need to fix it.
> 
> This fixes the "modeset-non-lpsp" test from both "pc8" and
> "runtime_pm" tests from intel-gpu-tools.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a0c907f..5b50083 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5547,6 +5547,8 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
>  	bool is_enabled, enable_requested;
>  	uint32_t tmp;
>  
> +	WARN_ON(dev_priv->pc8.enabled);
> +
>  	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
>  	is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED;
>  	enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST;
> @@ -5591,16 +5593,24 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
>  static void __intel_power_well_get(struct drm_device *dev,
>  				   struct i915_power_well *power_well)
>  {
> -	if (!power_well->count++)
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (!power_well->count++) {
> +		hsw_disable_package_c8(dev_priv);
>  		__intel_set_power_well(dev, true);
> +	}
>  }
>  
>  static void __intel_power_well_put(struct drm_device *dev,
>  				   struct i915_power_well *power_well)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
>  	WARN_ON(!power_well->count);
> -	if (!--power_well->count)
> +	if (!--power_well->count) {
>  		__intel_set_power_well(dev, false);
> +		hsw_enable_package_c8(dev_priv);
> +	}
>  }

It would be better to add these __intel_set_power_well(), which has all
the HSW specific bits already.

--Imre

>  
>  void intel_display_power_get(struct drm_device *dev,


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx