[Intel-gfx] [PATCH 5/5] drm/i915/vlv: support save/restore of display state around power well toggle

[Paulo Zanoni]

2013/10/14 Jesse Barnes <jbarnes at virtuousgeek.org>:
> If we disable the power well at runtime, we need to save enough display
> state so we can restore it when the power well comes back again.  Add
> support for that on VLV by reusing some of the _freeze and _thaw code.
>
> Note we need to drop the power well lock in this path around the restore,
> since we'll end up in mode set functions that take more refs on the
> power well.
>
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index b126f5a..070ff00 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -406,7 +406,26 @@ static void __vlv_set_power_well(struct drm_i915_private *dev_priv,
>  static void vlv_set_display_power(struct drm_i915_private *dev_priv,
>                                   bool enable)
>  {
> -       __vlv_set_power_well(dev_priv, DISP2D_PWRGT, enable);
> +       struct drm_device *dev = dev_priv->dev;
> +       struct i915_power_well *power_well = &dev_priv->power_well;
> +
> +       if (enable) {
> +               /* Lost all the display state, restore it */
> +               if (vlv_display_power_enabled(dev_priv))
> +                       return; /* already on, skip the fireworks */
> +               __vlv_set_power_well(dev_priv, DISP2D_PWRGT, true);
> +               spin_unlock_irq(&power_well->lock);
> +               i915_restore_state(dev);

At least on Haswell, i915_restore_state restores a lot more than just
what was lost on the power well. Do you really need all this? Besides,
I kinda fear you can break things by restoring stuff that is already
running. While we're doing this restoring, interrputs are happening,
everything is running, yet we call stuff like intel_i2c_reset,
i915_gem_restore_fences and others. Another example: we have code to
restore the backlight registers, but backlight is usually on the
output that works even with the power well disabled. So if we disable
the power well, then change backlight on the only output that is
working, then reenable the power well we'll "restore" a bogus
backlight state on a case where we shouldn't be touching backlight at
all.

> +               intel_modeset_init_hw(dev);
> +               intel_modeset_setup_hw_state(dev, true);
> +               spin_lock_irq(&power_well->lock);
> +       } else {
> +               if (!vlv_display_power_enabled(dev_priv))
> +                       return; /* already off, skip the fireworks */
> +               /* Make sure we save the state we need */
> +               i915_save_state(dev);
> +               __vlv_set_power_well(dev_priv, DISP2D_PWRGT, enable);
> +       }
>  }
>
>  static void vlv_set_render_power(struct drm_i915_private *dev_priv, bool enable)
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni