[Intel-gfx] [PATCH 14/15] drm/i915: vlv: add runtime PM support
Imre Deak
imre.deak at intel.com
Wed Apr 9 17:43:10 CEST 2014
On Wed, 2014-04-09 at 16:22 +0200, Daniel Vetter wrote:
> On Tue, Apr 08, 2014 at 07:57:55PM +0300, Imre Deak wrote:
> > Add runtime PM support for VLV, but leave it disabled. The next patch
> > enables it.
> >
> > The suspend/resume sequence used is based on [1] and [2]. In practice we
> > depend on the GT RC6 mechanism to save the HW context depending on the
> > render and media power wells. By the time we run the runtime suspend
> > callback the display side is also off and the HW context for that is
> > managed by the display power domain framework.
> >
> > Besides the above there are Gunit registers that depend on a system-wide
> > power well. This power well goes off once the device enters any of the
> > S0i[R123] states. To handle this scenario, save/restore these Gunit
> > registers. Note that this is not the complete register set dictated by
> > [2], to remove some overhead registers that are known not to be used are
> > ignored. Also some registers are fully setup by initialization functions
> > called during resume, these are not saved either. The list of registers
> > can be further reduced, see the TODO note in the code.
> >
> > [1] VLV_gfx_clocking_PM_reset_y12w21d3 / "Driver D3 entry/exit"
> > [2] VLV2_S0IXRegs
> >
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 170 +++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 166 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 9f9d0db..16ca37f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -891,19 +891,23 @@ static int i915_pm_poweroff(struct device *dev)
> > return i915_drm_freeze(drm_dev);
> > }
> >
> > -static void snb_runtime_suspend(struct drm_i915_private *dev_priv)
> > +static int snb_runtime_suspend(struct drm_i915_private *dev_priv)
> > {
> > struct drm_device *dev = dev_priv->dev;
> >
> > intel_runtime_pm_disable_interrupts(dev);
> > +
> > + return 0;
> > }
> >
> > -static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
> > +static int hsw_runtime_suspend(struct drm_i915_private *dev_priv)
> > {
> > hsw_enable_pc8(dev_priv);
> > +
> > + return 0;
> > }
> >
> > -static void snb_runtime_resume(struct drm_i915_private *dev_priv)
> > +static int snb_runtime_resume(struct drm_i915_private *dev_priv)
> > {
> > struct drm_device *dev = dev_priv->dev;
> >
> > @@ -913,11 +917,15 @@ static void snb_runtime_resume(struct drm_i915_private *dev_priv)
> > mutex_lock(&dev_priv->rps.hw_lock);
> > gen6_update_ring_freq(dev);
> > mutex_unlock(&dev_priv->rps.hw_lock);
> > +
> > + return 0;
> > }
> >
> > -static void hsw_runtime_resume(struct drm_i915_private *dev_priv)
> > +static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
> > {
> > hsw_disable_pc8(dev_priv);
> > +
> > + return 0;
> > }
> >
> > /*
> > @@ -1144,11 +1152,155 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
> > #undef COND
> > }
> >
> > +static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
> > +{
> > + u32 val;
> > + int err = 0;
> > +
> > + val = I915_READ(VLV_GTLC_WAKE_CTRL);
> > + val &= ~VLV_GTLC_ALLOWWAKEREQ;
> > + if (allow)
> > + val |= VLV_GTLC_ALLOWWAKEREQ;
> > + I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
> > + POSTING_READ(VLV_GTLC_WAKE_CTRL);
> > +
> > +#define COND (!!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEACK) == \
> > + allow)
> > + err = wait_for(COND, 1);
> > + if (err)
> > + DRM_ERROR("timeout disabling GT waking\n");
> > + return err;
> > +#undef COND
> > +}
> > +
> > +static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
> > + bool wait_for_on)
> > +{
> > + u32 mask;
> > + u32 val;
> > + int err;
> > +
> > + mask = VLV_GTLC_PW_MEDIA_STATUS_MASK | VLV_GTLC_PW_RENDER_STATUS_MASK;
> > + val = wait_for_on ? mask : 0;
> > +#define COND ((I915_READ(VLV_GTLC_PW_STATUS) & mask) == val)
> > + if (COND)
> > + return 0;
> > +
> > + DRM_DEBUG_KMS("waiting for GT wells to go %s (%08x)\n",
> > + wait_for_on ? "on" : "off",
> > + I915_READ(VLV_GTLC_PW_STATUS));
> > +
> > + /*
> > + * RC6 transitioning can be delayed up to 2 msec (see
> > + * valleyview_enable_rps), use 3 msec for safety.
> > + */
> > + err = wait_for(COND, 3);
> > + if (err)
> > + DRM_ERROR("timeout waiting for GT wells to go %s\n",
> > + wait_for_on ? "on" : "off");
> > +
> > + return err;
> > +#undef COND
> > +}
> > +
> > +static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
> > +{
> > + if (!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEERR))
> > + return;
> > +
> > + DRM_ERROR("GT register access while GT waking disabled\n");
> > + I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
> > +}
> > +
> > +static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
> > +{
> > + struct drm_device *dev = dev_priv->dev;
> > + u32 mask;
> > + int err;
> > +
> > + if (WARN_ON(!valleyview_rc6_enabled(dev)))
> > + return -ENODEV;
> > +
> > + intel_runtime_pm_disable_interrupts(dev);
> > + cancel_work_sync(&dev_priv->rps.work);
> > +
> > + /*
> > + * Bspec defines the following GT well on flags as debug only, so
> > + * don't treat them as hard failures.
> > + */
> > + (void)vlv_wait_for_gt_wells(dev_priv, false);
> > +
> > + mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS;
> > + WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL) & mask) != mask);
> > +
> > + vlv_check_no_gt_access(dev_priv);
> > +
> > + err = vlv_force_gfx_clock(dev_priv, true);
> > + if (err)
> > + goto err1;
> > +
> > + err = vlv_allow_gt_wake(dev_priv, false);
> > + if (err)
> > + goto err2;
> > + vlv_save_gunit_s0ix_state(dev_priv);
> > +
> > + err = vlv_force_gfx_clock(dev_priv, false);
> > + if (err)
> > + goto err2;
> > +
> > + return 0;
> > +
> > +err2:
> > + /* For safety always re-enable waking and disable gfx clock forcing */
> > + vlv_allow_gt_wake(dev_priv, true);
> > +err1:
> > + vlv_force_gfx_clock(dev_priv, false);
> > + intel_runtime_pm_restore_interrupts(dev);
> > +
> > + return err;
> > +}
> > +
> > +static int vlv_runtime_resume(struct drm_i915_private *dev_priv)
> > +{
> > + struct drm_device *dev = dev_priv->dev;
> > + int err;
> > + int ret;
> > +
> > + /*
> > + * If any of the steps fail just try to continue, that's the best we
> > + * can do at this point. Return the first error code (which will also
> > + * leave RPM permanentyl disabled).
> > + */
> > + ret = vlv_force_gfx_clock(dev_priv, true);
> > +
> > + vlv_restore_gunit_s0ix_state(dev_priv);
> > +
> > + err = vlv_allow_gt_wake(dev_priv, true);
> > + if (!ret)
> > + ret = err;
> > +
> > + err = vlv_force_gfx_clock(dev_priv, false);
> > + if (!ret)
> > + ret = err;
> > +
> > + vlv_check_no_gt_access(dev_priv);
> > +
> > + intel_init_clock_gating(dev);
> > + intel_reset_gt_powersave(dev);
> > + i915_gem_init_swizzling(dev);
> > + i915_gem_restore_fences(dev);
> > +
> > + intel_runtime_pm_restore_interrupts(dev);
> > +
> > + return ret;
> > +}
> > +
> > static int intel_runtime_suspend(struct device *device)
> > {
> > struct pci_dev *pdev = to_pci_dev(device);
> > struct drm_device *dev = pci_get_drvdata(pdev);
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > + int ret = 0;
> >
> > if (WARN_ON_ONCE(!dev_priv->rps.enabled))
> > return -ENODEV;
> > @@ -1162,9 +1314,17 @@ static int intel_runtime_suspend(struct device *device)
> > snb_runtime_suspend(dev_priv);
> > else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > hsw_runtime_suspend(dev_priv);
> > + else if (IS_VALLEYVIEW(dev))
> > + ret = vlv_runtime_suspend(dev_priv);
> > else
> > WARN_ON(1);
> >
> > + if (ret) {
> > + DRM_ERROR("Runtime suspend failed, disabling it\n");
> > +
> > + return ret;
> > + }
> > +
> > i915_gem_release_all_mmaps(dev_priv);
> >
> > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
> > @@ -1200,6 +1360,8 @@ static int intel_runtime_resume(struct device *device)
> > snb_runtime_resume(dev_priv);
> > else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > hsw_runtime_resume(dev_priv);
> > + else if (IS_VALLEYVIEW(dev))
> > + vlv_runtime_resume(dev_priv);
>
> Golden rule of refactoring: The 3rd guy gets to cleanup the mess. Imo
> it's time to refactor the common parts form these platform functions out
> and move them into generic code, and only call down into platform code as
> needed. If we don't do that we'll have completely hell due to slight
> differences in ordering between platforms.
Ok, the common parts basically boil down to
intel_runtime_pm_disable_interrupts(); / _enable_interrupts();
and I think we also need to add
cancel_work_sync(&dev_priv->rps.work);
to all platforms. I can do this.
> Also I think we should try to share as much code as possible with the
> other setup/teardowns paths, i.e. driver load/unload, system
> suspend/resume and gpu reset.
I agree, but this needs much more refactoring first on the other parts
you mention above before we can unify things. That's mainly because on
the RPM path we can call only low level functions (since an RPM callback
can be called basically from anywhere in the driver) whereas the
handlers you mention do a high level initialization. So for VLV RPM
resume we call for example
intel_init_clock_gating();
i915_gem_init_swizzling();
i915_gem_restore_fences();
but for system resume we simply do a full
intel_modeset_init_hw();
which includes the above low level steps interleaved with quite a lot of
other init steps. We may also need to rethink locking before sharing
those parts.
So I'm happy to do this refactoring, but I'd suggest doing it as a
follow-up.
--Imre
> Thanks, Daniel
>
> > else
> > WARN_ON(1);
> >
> > --
> > 1.8.4
> >
> > _______________________________________________
> > 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