[Intel-gfx] [PATCH v2 04/16] drm/i915/gen9: Reset secondary power well requests left on by DMC/KVMR

[Patrik Jakobsson]

On Tue, Apr 05, 2016 at 01:26:05PM +0300, Imre Deak wrote:
> DMC forces on power well 1 and the misc IO power well by setting the
> corresponding request bits both in the BIOS and the DEBUG power well
> request registers. This is somewhat unexpected since the firmware should
> really just save and restore state but not alter it. We also depend on
> being able to disable power well 1, and the misc IO power well before
> entering S3/S4 on BXT and SKL or entering DC9 on BXT. To fix this make
> sure these request bits are cleared whenever we want to disable the
> given power wells.
> 
> On SKL there is another twist where the firmware also clears the power
> well 1 request bit in HSW_POWER_WELL_DRIVER (but not that of the misc IO
> power well). This happens to not cause a problem due to the forced-on
> request bits in the other request registers.
> 
> I've filed a bug about all this, but fixing that may take a while and
> having this sanity check in place makes sense even for future firmware
> versions.
> 
> At the same time also check the KVMR request bits. I haven't seen this
> being altered, but we don't expect any request bits in here either, so
> sanitize this register as well.
> 
> v2:
> - apply the workaround on SKL as well
> 
> CC: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

Hmm, more DMC fun.

Reviewed-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 41 +++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index d189a00..6ffa6ad 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -630,6 +630,44 @@ void skl_disable_dc6(struct drm_i915_private *dev_priv)
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  }
>  
> +static void
> +gen9_sanitize_power_well_requests(struct drm_i915_private *dev_priv,
> +				  struct i915_power_well *power_well)
> +{
> +	enum skl_disp_power_wells power_well_id = power_well->data;
> +	u32 val;
> +	u32 mask;
> +
> +	mask = SKL_POWER_WELL_REQ(power_well_id);
> +
> +	val = I915_READ(HSW_PWR_WELL_KVMR);
> +	if (WARN_ONCE(val & mask, "Clearing unexpected KVMR request for %s\n",
> +				   power_well->name))
> +		I915_WRITE(HSW_PWR_WELL_KVMR, val & ~mask);
> +
> +	val = I915_READ(HSW_PWR_WELL_BIOS);
> +	val |= I915_READ(HSW_PWR_WELL_DEBUG);
> +
> +	if (!(val & mask))
> +		return;
> +
> +	/*
> +	 * DMC is known to force on the request bits for power well 1 on SKL
> +	 * and BXT and the misc IO power well on SKL but we don't expect any
> +	 * other request bits to be set, so WARN for those.
> +	 */
> +	if (power_well_id == SKL_DISP_PW_1 ||
> +	    (IS_SKYLAKE(dev_priv) && power_well_id == SKL_DISP_PW_MISC_IO))
> +		DRM_DEBUG_DRIVER("Clearing auxiliary requests for %s forced on "
> +				 "by DMC\n", power_well->name);
> +	else
> +		WARN_ONCE(1, "Clearing unexpected auxiliary requests for %s\n",
> +			  power_well->name);
> +
> +	I915_WRITE(HSW_PWR_WELL_BIOS, val & ~mask);
> +	I915_WRITE(HSW_PWR_WELL_DEBUG, val & ~mask);
> +}
> +
>  static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  			struct i915_power_well *power_well, bool enable)
>  {
> @@ -696,6 +734,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  			POSTING_READ(HSW_PWR_WELL_DRIVER);
>  			DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>  		}
> +
> +		if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> +			gen9_sanitize_power_well_requests(dev_priv, power_well);
>  	}
>  
>  	if (check_fuse_status) {
> -- 
> 2.5.0
> 

-- 
---
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027