[Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.

[Chris Wilson]

On Wed, Jul 17, 2013 at 12:36:12PM -0300, Rodrigo Vivi wrote:
> i915_powersave was already an umbrella for disabling downclocking and fbc.
> Now on it is extended to also force enabling them.
> Also more powersavings features has been added under it: RC6 and IPS.
> 
> In the future it can cover more powersavings features like
> PSR, Slice Shutdown, etc. So this will be the easiest path to disable or
> enable all of them together.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |  9 +++++++--
>  drivers/gpu/drm/i915/intel_display.c |  9 +++++----
>  drivers/gpu/drm/i915/intel_pm.c      | 13 ++++++-------
>  3 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b07362f..e4fb431 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -53,10 +53,15 @@ MODULE_PARM_DESC(panel_ignore_lid,
>  		"Override lid status (0=autodetect, 1=autodetect disabled [default], "
>  		"-1=force lid closed, -2=force lid open)");
>  
> -unsigned int i915_powersave __read_mostly = 1;
> +unsigned int i915_powersave __read_mostly = -1;
>  module_param_named(powersave, i915_powersave, int, 0600);
>  MODULE_PARM_DESC(powersave,
> -		"Enable powersavings, fbc, downclocking, etc. (default: true)");
> +		"Force Enable/Disable powersavings,"
> +		"ignoring individual parameters when available."
> +		"Features covered: downclocking, fbc, rc6 and ips"
> +		"0 = forcibly disables all powersavings features"
> +		"1 = forcibly enables all powersavings features"
> +		"default: -1 (use per-feature default/parameter)");
>  
>  int i915_semaphores __read_mostly = -1;
>  module_param_named(semaphores, i915_semaphores, int, 0600);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f53359b..e84a7d9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4089,7 +4089,8 @@ retry:
>  static void hsw_compute_ips_config(struct intel_crtc *crtc,
>  				   struct intel_crtc_config *pipe_config)
>  {
> -	pipe_config->ips_enabled = i915_enable_ips &&
> +	pipe_config->ips_enabled = i915_powersave != 0 &&
> +				   (i915_powersave == 1 || i915_enable_ips) &&
>  				   hsw_crtc_supports_ips(crtc) &&
>  				   pipe_config->pipe_bpp == 24;
>  }
> @@ -4329,7 +4330,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>  
>  	crtc->lowfreq_avail = false;
>  	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> -	    reduced_clock && i915_powersave) {
> +	    reduced_clock && i915_powersave == 0) {

Sense inverted.

> @@ -452,9 +452,6 @@ void intel_update_fbc(struct drm_device *dev)
>  	struct drm_i915_gem_object *obj;
>  	unsigned int max_hdisplay, max_vdisplay;
>  
> -	if (!i915_powersave)
> -		return;
> -
>  	if (!I915_HAS_FBC(dev))
>  		return;
>  
> @@ -491,13 +488,13 @@ void intel_update_fbc(struct drm_device *dev)
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> -	if (i915_enable_fbc < 0 &&
> +	if (i915_enable_fbc < 0 && i915_powersave < 0 &&
>  	    INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) {
>  		DRM_DEBUG_KMS("disabled per chip default\n");
>  		dev_priv->fbc.no_fbc_reason = FBC_CHIP_DEFAULT;
>  		goto out_disable;
>  	}
> -	if (!i915_enable_fbc) {
> +	if (!i915_enable_fbc || i915_powersave == 0) {

This is unreadable. Split it up like intel_enable_rc6() and make it
verbose.

>  		DRM_DEBUG_KMS("fbc disabled per module param\n");
>  		dev_priv->fbc.no_fbc_reason = FBC_MODULE_PARAM;
>  		goto out_disable;
> @@ -3171,12 +3168,14 @@ int intel_enable_rc6(const struct drm_device *dev)
>  	if (INTEL_INFO(dev)->gen < 5)
>  		return 0;
>  
> -	/* Respect the kernel parameter if it is set */
> +	/* Respect the kernel parameters if they are set */
> +	if (i915_powersave == 0)
> +		return 0;

Semantics broken here. Force enabling a specific tunable such as rc6
should override a default powersave.

>  	if (i915_enable_rc6 >= 0)
>  		return i915_enable_rc6;
>  
>  	/* Disable RC6 on Ironlake */
> -	if (INTEL_INFO(dev)->gen == 5)
> +	if (INTEL_INFO(dev)->gen == 5 && i915_powersave < 0)
>  		return 0;

-- 
Chris Wilson, Intel Open Source Technology Centre