[Intel-gfx] [PATCH] drm/i915/skl: enable PC9/10 power states during suspend-to-idle
Imre Deak
imre.deak at intel.com
Wed Nov 18 08:44:43 PST 2015
On ke, 2015-11-18 at 17:33 +0100, Daniel Vetter wrote:
> On Wed, Nov 18, 2015 at 05:32:30PM +0200, Imre Deak wrote:
> > During suspend-to-idle we need to keep the DMC firmware active and DC6
> > enabled, since otherwise we won't reach deep system power states like
> > PC9/10. The lead for this came from Nivedita who noticed that the
> > kernel's turbostat tool didn't report any PC9/10 residency change
> > across an 'echo freeze > /sys/power/state'.
> >
> > Reported-by: Nivedita Swaminathan <nivedita.swaminathan at intel.com>
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 44 +++++++++++++++++++++++++++++++----------
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > 2 files changed, 35 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 6344dfb..649e20a 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -624,6 +624,14 @@ static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
> > bool rpm_resume);
> > static int bxt_resume_prepare(struct drm_i915_private *dev_priv);
> >
> > +static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > +{
> > +#if IS_ENABLED(CONFIG_ACPI_SLEEP)
> > + if (acpi_target_system_state() < ACPI_STATE_S3)
> > + return true;
> > +#endif
> > + return false;
> > +}
> >
> > static int i915_drm_suspend(struct drm_device *dev)
> > {
> > @@ -676,11 +684,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> >
> > i915_save_state(dev);
> >
> > - opregion_target_state = PCI_D3cold;
> > -#if IS_ENABLED(CONFIG_ACPI_SLEEP)
> > - if (acpi_target_system_state() < ACPI_STATE_S3)
> > - opregion_target_state = PCI_D1;
> > -#endif
> > + opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
> > intel_opregion_notify_adapter(dev, opregion_target_state);
> >
> > intel_uncore_forcewake_reset(dev, false);
> > @@ -701,15 +705,26 @@ static int i915_drm_suspend(struct drm_device *dev)
> > static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
> > {
> > struct drm_i915_private *dev_priv = drm_dev->dev_private;
> > + bool fw_csr;
> > int ret;
> >
> > - intel_power_domains_suspend(dev_priv);
> > + fw_csr = suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
> > + /*
> > + * In case of firmware assisted context save/restore don't manually
> > + * deinit the power domains. This also means the CSR/DMC firmware will
> > + * stay active, it will power down any HW resources as required and
> > + * also enable deeper system power states that would be blocked if the
> > + * firmware was inactive.
> > + */
> > + if (!fw_csr)
> > + intel_power_domains_suspend(dev_priv);
> >
> > ret = intel_suspend_complete(dev_priv);
> >
> > if (ret) {
> > DRM_ERROR("Suspend complete failed: %d\n", ret);
> > - intel_power_domains_init_hw(dev_priv, true);
> > + if (!fw_csr)
> > + intel_power_domains_init_hw(dev_priv, true);
> >
> > return ret;
> > }
> > @@ -730,6 +745,8 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
> > if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6))
> > pci_set_power_state(drm_dev->pdev, PCI_D3hot);
> >
> > + dev_priv->suspended_to_idle = suspend_to_idle(dev_priv);
> > +
> > return 0;
> > }
> >
> > @@ -842,8 +859,10 @@ static int i915_drm_resume_early(struct drm_device *dev)
> > * FIXME: This should be solved with a special hdmi sink device or
> > * similar so that power domains can be employed.
> > */
> > - if (pci_enable_device(dev->pdev))
> > - return -EIO;
> > + if (pci_enable_device(dev->pdev)) {
> > + ret = -EIO;
> > + goto out;
> > + }
> >
> > pci_set_master(dev->pdev);
> >
> > @@ -861,7 +880,12 @@ static int i915_drm_resume_early(struct drm_device *dev)
> > hsw_disable_pc8(dev_priv);
> >
> > intel_uncore_sanitize(dev);
> > - intel_power_domains_init_hw(dev_priv, true);
> > +
> > + if (!(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload))
> > + intel_power_domains_init_hw(dev_priv, true);
>
> This doesn't work when e.g. system suspend to S3 fails, in which case dmc
> will also survive. We need the pm core to tell us what really happened,
> and Rafael Wyzocki was working on patches for that:
>
> http://lkml.iu.edu/hypermail/linux/kernel/1510.0/04266.html
>
> Specifically we need the pm_suspend/resume_via_firmware helpers.
> Unfortunately these didn't make it into 4.4, so we need to pick Rafael.
I think it does work. We deactivate the firmware during S3,S4 suspend
in any case and reprogram it during S3,S4 resume, so it doesn't matter
if the firmware contents survive or not. What we want is to ensure that
during suspend-to-idle we don't deactivate the firmware that is we
don't disable DC6 and we can decide that based
on acpi_target_system_state().
--Imre
> Cheers, Daniel
>
> > +
> > +out:
> > + dev_priv->suspended_to_idle = false;
> >
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index a32571f..4a50382 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1882,6 +1882,7 @@ struct drm_i915_private {
> > u32 chv_phy_control;
> >
> > u32 suspend_count;
> > + bool suspended_to_idle;
> > struct i915_suspend_saved_registers regfile;
> > struct vlv_s0ix_state vlv_s0ix_state;
> >
> > --
> > 2.5.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
More information about the Intel-gfx
mailing list