[Intel-gfx] [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states

[Patrik Jakobsson]

On Mon, Feb 29, 2016 at 10:49:03PM +0200, Imre Deak wrote:
> We can simplify the conditions selecting the target DC state during
> runtime by calculating the allowed DC states in advance during driver
> loading. This also makes it easier to disable DC states depending on the
> i915.disable_power_well module option, added in the next patch.
> 
> v2:
> - Print a debug message if the requested max DC value was adjusted due
>   to a platform limit. Also debug print the calculated mask value. (Patrik)
> 
> CC: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 78 ++++++++++++++++++++++++---------
>  2 files changed, 58 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6712955..dc554dd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -754,6 +754,7 @@ struct intel_csr {
>  	i915_reg_t mmioaddr[8];
>  	uint32_t mmiodata[8];
>  	uint32_t dc_state;
> +	uint32_t allowed_dc_mask;
>  };
>  
>  #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6e54d97..30df9de 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -538,12 +538,8 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
>  	else
>  		mask |= DC_STATE_EN_UPTO_DC6;
>  
> -	WARN_ON_ONCE(state & ~mask);
> -
> -	if (i915.enable_dc == 0)
> -		state = DC_STATE_DISABLE;
> -	else if (i915.enable_dc == 1 && state > DC_STATE_EN_UPTO_DC5)
> -		state = DC_STATE_EN_UPTO_DC5;
> +	if (WARN_ON_ONCE(state & ~dev_priv->csr.allowed_dc_mask))
> +		state &= dev_priv->csr.allowed_dc_mask;
>  
>  	val = I915_READ(DC_STATE_EN);
>  	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n",
> @@ -659,8 +655,7 @@ static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
>  {
>  	assert_can_disable_dc5(dev_priv);
>  
> -	if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> -	    i915.enable_dc != 0 && i915.enable_dc != 1)
> +	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
>  		assert_can_disable_dc6(dev_priv);
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> @@ -839,26 +834,19 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>  static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> -	if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> -	    i915.enable_dc != 0 && i915.enable_dc != 1)
> +	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
>  		skl_enable_dc6(dev_priv);
> -	else
> +	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
>  		gen9_enable_dc5(dev_priv);
>  }
>  
>  static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> -	if (power_well->count > 0) {
> -		gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> -	} else {
> -		if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> -		    i915.enable_dc != 0 &&
> -		    i915.enable_dc != 1)
> -			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
> -		else
> -			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
> -	}
> +	if (power_well->count > 0)
> +		gen9_dc_off_power_well_enable(dev_priv, power_well);
> +	else
> +		gen9_dc_off_power_well_disable(dev_priv, power_well);
>  }
>  
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
> @@ -2023,6 +2011,52 @@ sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv,
>  	return 1;
>  }
>  
> +static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> +				    int enable_dc)
> +{
> +	uint32_t mask;
> +	int requested_dc;
> +	int max_dc;
> +
> +	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> +		max_dc = 2;
> +		mask = 0;
> +	} else if (IS_BROXTON(dev_priv)) {
> +		max_dc = 1;
> +		/*
> +		 * DC9 has a separate HW flow from the rest of the DC states,
> +		 * not depending on the DMC firmware. It's needed by system
> +		 * suspend/resume, so allow it unconditionally.
> +		 */
> +		mask = DC_STATE_EN_DC9;
> +	} else {
> +		max_dc = 0;
> +		mask = 0;
> +	}
> +
> +	if (enable_dc >= 0 && enable_dc <= max_dc) {
> +		requested_dc = enable_dc;
> +	} else if (enable_dc == -1) {
> +		requested_dc = max_dc;
> +	} else if (enable_dc > max_dc && enable_dc <= 2) {
> +		DRM_DEBUG_KMS("Adjusting requested max DC state (%d->%d)\n",
> +			      enable_dc, max_dc);
> +		requested_dc = max_dc;
> +	} else {
> +		DRM_ERROR("Unexpected value for enable_dc (%d)\n", enable_dc);
> +		requested_dc = max_dc;
> +	}
> +
> +	if (requested_dc > 1)
> +		mask |= DC_STATE_EN_UPTO_DC6;
> +	if (requested_dc > 0)
> +		mask |= DC_STATE_EN_UPTO_DC5;
> +
> +	DRM_DEBUG_KMS("Allowed DC state mask %02x\n", mask);
> +
> +	return mask;
> +}
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -2041,6 +2075,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  
>  	i915.disable_power_well = sanitize_disable_power_well_option(dev_priv,
>  						     i915.disable_power_well);
> +	dev_priv->csr.allowed_dc_mask = get_allowed_dc_mask(dev_priv,
> +							    i915.enable_dc);
>  
>  	BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
>  
> -- 
> 2.5.0
> 

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