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

[Rodrigo Vivi]

when playing with fbc and psr I noticed that this flag was underused
so I decided to extend it a bit...
I know that the force enable made it more difficult to implement along
with individual parameters, but I sent this patch as a startup for the
discussion...

please let me know if you have better ideas...

On Wed, Jul 17, 2013 at 12:36 PM, Rodrigo Vivi <rodrigo.vivi at gmail.com> 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) {
>                 I915_WRITE(FP1(pipe), fp2);
>                 crtc->config.dpll_hw_state.fp1 = fp2;
>                 crtc->lowfreq_avail = true;
> @@ -7156,7 +7157,7 @@ void intel_mark_idle(struct drm_device *dev)
>  {
>         struct drm_crtc *crtc;
>
> -       if (!i915_powersave)
> +       if (i915_powersave == 0)
>                 return;
>
>         list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> @@ -7173,7 +7174,7 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
>         struct drm_device *dev = obj->base.dev;
>         struct drm_crtc *crtc;
>
> -       if (!i915_powersave)
> +       if (i915_powersave == 0)
>                 return;
>
>         list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 43031ec..405b475 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -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) {
>                 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;
>         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;
>
>         if (IS_HASWELL(dev)) {
> --
> 1.8.1.4
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br