[Intel-gfx] [PATCH v3 11/12] drm/i915/gen9: Add boot parameter for disabling DC6

[Imre Deak]

On ma, 2015-11-16 at 15:01 +0100, Patrik Jakobsson wrote:
> v2: Use _unsafe (Jani)
> v3: Allow specifying specific DC-states instead of just DC6 (Imre)
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_params.c      |  6 ++++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 14 +++++++++++---
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c0252ef..c85c1ec 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2639,6 +2639,7 @@ struct i915_params {
>  	int panel_use_ssc;
>  	int vbt_sdvo_panel_type;
>  	int enable_rc6;
> +	int enable_dc;
>  	int enable_fbc;
>  	int enable_ppgtt;
>  	int enable_execlists;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 368df67..ccc15c9 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
>  	.panel_use_ssc = -1,
>  	.vbt_sdvo_panel_type = -1,
>  	.enable_rc6 = -1,
> +	.enable_dc = -1,
>  	.enable_fbc = -1,
>  	.enable_execlists = -1,
>  	.enable_hangcheck = true,
> @@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
>  	"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
>  	"default: -1 (use per-chip default)");
>  
> +module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400);
> +MODULE_PARM_DESC(enable_dc,
> +	"Enable power-saving display C-states. "
> +	"(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
> +
>  module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
>  MODULE_PARM_DESC(enable_fbc,
>  	"Enable frame buffer compression for power savings "
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b6ee48e..4d6f777 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -417,6 +417,11 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
>  
>  	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;
> +

Ideally we would have a sanitize function for the option and setup a
dev_priv->enable_dc_state there based on the platform and the option
value. Then the check for which state to enable would be simpler and we
could have the check only in the power well enable/disable/sync_hw
hooks, and set the passed in state here unconditionally. But this can
be done as a follow-up, this patch looks ok to me regardless:

Reviewed-by: Imre Deak <imre.deak at intel.com>

>  	if (state & DC_STATE_EN_UPTO_DC5_DC6_MASK)
>  		gen9_set_dc_state_debugmask_memory_up(dev_priv);
>  
> @@ -524,7 +529,9 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
>  static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
>  {
>  	assert_can_disable_dc5(dev_priv);
> -	assert_can_disable_dc6(dev_priv);
> +
> +	if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 && i915.enable_dc != 1)
> +		assert_can_disable_dc6(dev_priv);
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  }
> @@ -709,7 +716,7 @@ 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))
> +	if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 && i915.enable_dc != 1)
>  		skl_enable_dc6(dev_priv);
>  	else
>  		gen9_enable_dc5(dev_priv);
> @@ -721,7 +728,8 @@ static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  	if (power_well->count > 0) {
>  		gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  	} else {
> -		if (IS_SKYLAKE(dev_priv))
> +		if (IS_SKYLAKE(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);