On Tue, 15 Oct 2013 17:09:19 -0300
Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> 2013/10/14 Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>:
> > 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@xxxxxxxxxxxxxxxx>
> > ---
> > 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.
Yeah this is definitely a WIP. We'll probably need power well specific
save/restore functions. Depending on which well was toggled, the state
that needs to be saved & restored will be different. It may be best to
try to push all this into modeset_setup_hw_state, since it should know
which power wells are needed for different ops and thus restore the
right state. But that means ripping apart save/restore_state and
putting bits in the right places, potentially with specific hooks like
the uncore save/restore I added for other stuff.
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
