| Message ID | 20170706192450.28477-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 06, 2017 at 09:24:49PM +0200, Hans de Goede wrote: > Quoting Ville: "the forcewake timer might still be active until the uncore > suspend, and having active forcewakes while we've already told the GT wake > stuff to stop acting normally doesn't seem quite right to me." > > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Suggested-by: Imre Deak <imre.deak@intel.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Rebase on current (July 6th 2017) drm-next > --- > drivers/gpu/drm/i915/i915_drv.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index ce31d9ed23dc..4a6cd3176e0a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2415,6 +2415,8 @@ static int intel_runtime_suspend(struct device *kdev) > > intel_runtime_pm_disable_interrupts(dev_priv); > > + intel_uncore_suspend(dev_priv); > + Yep, this fixes a problem on VLV independent of your IOSF/forcewake fix. vlv_suspend_complete() calls vlv_allow_gt_wake(false) after which we shouldn't have any pending forcewakes until resume. AFAICS after this point there isn't anything requiring forcewake until resume so the change looks ok: Reviewed-by: Imre Deak <imre.deak@intel.com> > ret = 0; > if (IS_GEN9_LP(dev_priv)) { > bxt_display_core_uninit(dev_priv); > @@ -2427,6 +2429,8 @@ static int intel_runtime_suspend(struct device *kdev) > > if (ret) { > DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret); > + intel_uncore_runtime_resume(dev_priv); > + > intel_runtime_pm_enable_interrupts(dev_priv); > > enable_rpm_wakeref_asserts(dev_priv); > @@ -2434,8 +2438,6 @@ static int intel_runtime_suspend(struct device *kdev) > return ret; > } > > - intel_uncore_suspend(dev_priv); > - > enable_rpm_wakeref_asserts(dev_priv); > WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count)); > > -- > 2.13.0 >
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ce31d9ed23dc..4a6cd3176e0a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2415,6 +2415,8 @@ static int intel_runtime_suspend(struct device *kdev) intel_runtime_pm_disable_interrupts(dev_priv); + intel_uncore_suspend(dev_priv); + ret = 0; if (IS_GEN9_LP(dev_priv)) { bxt_display_core_uninit(dev_priv); @@ -2427,6 +2429,8 @@ static int intel_runtime_suspend(struct device *kdev) if (ret) { DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret); + intel_uncore_runtime_resume(dev_priv); + intel_runtime_pm_enable_interrupts(dev_priv); enable_rpm_wakeref_asserts(dev_priv); @@ -2434,8 +2438,6 @@ static int intel_runtime_suspend(struct device *kdev) return ret; } - intel_uncore_suspend(dev_priv); - enable_rpm_wakeref_asserts(dev_priv); WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count));
Quoting Ville: "the forcewake timer might still be active until the uncore suspend, and having active forcewakes while we've already told the GT wake stuff to stop acting normally doesn't seem quite right to me." Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Suggested-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: -Rebase on current (July 6th 2017) drm-next --- drivers/gpu/drm/i915/i915_drv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)