On Fri, 2014-04-25 at 09:59 +0200, Daniel Vetter wrote:
> On Mon, Apr 14, 2014 at 08:24:29PM +0300, Imre Deak wrote:
> > At least on VLV but probably on other platforms too we depend on RC6
> > being enabled for RPM, so disable RPM until the delayed RC6 enabling
> > completes.
> >
> > v2:
> > - explain the reason for the _noresume version of RPM get (Daniel)
> > - use the simpler 'if (schedule_work()) rpm_get();' instead of
> > 'if (!cancel_work_sync()) rpm_get(); schedule_work();'
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 5 ++++-
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++++++++++++++++++++--
> > 3 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index a821608..a20d2d1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -754,7 +754,7 @@ int i915_reset(struct drm_device *dev)
> > * previous concerns that it doesn't respond well to some forms
> > * of re-init after reset. */
> > if (INTEL_INFO(dev)->gen > 5)
> > - intel_enable_gt_powersave(dev);
> > + intel_reset_gt_powersave(dev);
> >
> > intel_hpd_init(dev);
> > } else {
> > @@ -923,6 +923,9 @@ static int intel_runtime_suspend(struct device *device)
> > struct drm_device *dev = pci_get_drvdata(pdev);
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > + if (WARN_ON_ONCE(!dev_priv->rps.enabled))
> > + return -ENODEV;
> > +
> > WARN_ON(!HAS_RUNTIME_PM(dev));
> > assert_force_wake_inactive(dev_priv);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index c551472..618d05a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -924,6 +924,7 @@ void intel_init_gt_powersave(struct drm_device *dev);
> > void intel_cleanup_gt_powersave(struct drm_device *dev);
> > void intel_enable_gt_powersave(struct drm_device *dev);
> > void intel_disable_gt_powersave(struct drm_device *dev);
> > +void intel_reset_gt_powersave(struct drm_device *dev);
> > void ironlake_teardown_rc6(struct drm_device *dev);
> > void gen6_update_ring_freq(struct drm_device *dev);
> > void gen6_rps_idle(struct drm_i915_private *dev_priv);
> > @@ -931,6 +932,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv);
> > void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
> > void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
> > void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> > +void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
> > void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
> > void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
> > void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index e5b9f08..0e8b263 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4549,6 +4549,8 @@ static void intel_gen6_powersave_work(struct work_struct *work)
> > }
> > dev_priv->rps.enabled = true;
> > mutex_unlock(&dev_priv->rps.hw_lock);
> > +
> > + intel_runtime_pm_put(dev_priv);
> > }
> >
> > void intel_enable_gt_powersave(struct drm_device *dev)
> > @@ -4566,12 +4568,28 @@ void intel_enable_gt_powersave(struct drm_device *dev)
> > * PCU communication is slow and this doesn't need to be
> > * done at any specific time, so do this out of our fast path
> > * to make resume and init faster.
> > + *
> > + * We depend on the HW RC6 power context save/restore
> > + * mechanism when entering D3 through runtime PM suspend. So
> > + * disable RPM until RPS/RC6 is properly setup. We can only
> > + * get here via the driver load/system resume/runtime resume
> > + * paths, so the _noresume version is enough (and in case of
> > + * runtime resume it's necessary).
> > */
> > - schedule_delayed_work(&dev_priv->rps.delayed_resume_work,
> > - round_jiffies_up_relative(HZ));
> > + if (schedule_delayed_work(&dev_priv->rps.delayed_resume_work,
> > + round_jiffies_up_relative(HZ)))
> > + intel_runtime_pm_get_noresume(dev_priv);
> > }
> > }
> >
> > +void intel_reset_gt_powersave(struct drm_device *dev)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > + dev_priv->rps.enabled = false;
> > + intel_enable_gt_powersave(dev);
> > +}
> > +
> > static void ibx_init_clock_gating(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -6025,6 +6043,18 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
> > WARN(dev_priv->pm.suspended, "Device still suspended.\n");
> > }
> >
> > +void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv)
> > +{
> > + struct drm_device *dev = dev_priv->dev;
> > + struct device *device = &dev->pdev->dev;
> > +
> > + if (!HAS_RUNTIME_PM(dev))
> > + return;
> > +
> > + WARN(dev_priv->pm.suspended, "Getting nosync-ref while suspended.\n");
>
> Do we really need our own tracking in dev_priv->pm.suspended, is there
> nothing in the runtime pm core which we could use instaed?
Hm, yes by a quick look pm_runtime_suspended() seems like what we could
use instead. But this was just copy&paste, and the rest of the spots
would need changing too, so could we do it as a follow-up?
--Imre
> -Daniel
>
> > + pm_runtime_get_noresume(device);
> > +}
> > +
> > void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
> > {
> > struct drm_device *dev = dev_priv->dev;
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
