| Message ID | 1394786834-9734-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 14, 2014 at 08:47:14AM +0000, Chris Wilson wrote: > Daniel stopped that we lose track of the user forcewake across suspend > and resume, so hook up the code to restore it. > > Reported-by: Daniel Vetter <daniel.veter@ffwll.ch> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.veter@ffwll.ch> Testcase: igt/drv_suspend/forcewake? Or have I screwed it up and the test is bonkers? -Daniel > --- > drivers/gpu/drm/i915/i915_drv.c | 5 +++-- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_uncore.c | 6 ++++++ > 3 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 3fbf8aa8d119..aa03af611ae8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -544,8 +544,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) > int error = 0; > > intel_uncore_early_sanitize(dev); > - > - intel_uncore_sanitize(dev); > + intel_uncore_restore(dev); > > if (drm_core_check_feature(dev, DRIVER_MODESET) && > restore_gtt_mappings) { > @@ -879,6 +878,8 @@ static int i915_runtime_resume(struct device *device) > intel_opregion_notify_adapter(dev, PCI_D0); > dev_priv->pm.suspended = false; > > + intel_uncore_restore(dev); > + > return 0; > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e63cfc88c178..c0990eeabd49 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2189,6 +2189,7 @@ extern void intel_uncore_sanitize(struct drm_device *dev); > extern void intel_uncore_early_sanitize(struct drm_device *dev); > extern void intel_uncore_init(struct drm_device *dev); > extern void intel_uncore_check_errors(struct drm_device *dev); > +extern void intel_uncore_restore(struct drm_device *dev); > extern void intel_uncore_fini(struct drm_device *dev); > > void > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 34edd7f22f17..e0bf0e54e340 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -831,6 +831,12 @@ void intel_uncore_init(struct drm_device *dev) > } > } > > +void intel_uncore_restore(struct drm_device *dev) > +{ > + intel_uncore_sanitize(dev); > + intel_uncore_forcewake_reset(dev, true); > +} > + > void intel_uncore_fini(struct drm_device *dev) > { > /* Paranoia: make sure we have disabled everything before we exit. */ > -- > 1.9.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Mar 14, 2014 at 04:51:56PM +0100, Daniel Vetter wrote: > On Fri, Mar 14, 2014 at 08:47:14AM +0000, Chris Wilson wrote: > > Daniel stopped that we lose track of the user forcewake across suspend > > and resume, so hook up the code to restore it. > > > > Reported-by: Daniel Vetter <daniel.veter@ffwll.ch> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.veter@ffwll.ch> > > Testcase: igt/drv_suspend/forcewake? Or have I screwed it up and the test > is bonkers? The testcase relies on the kernel exploding. We could try reading RING_CTL for a couple of seconds as see if returns 0. -Chris
On Fri, Mar 14, 2014 at 5:07 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Fri, Mar 14, 2014 at 04:51:56PM +0100, Daniel Vetter wrote: >> On Fri, Mar 14, 2014 at 08:47:14AM +0000, Chris Wilson wrote: >> > Daniel stopped that we lose track of the user forcewake across suspend >> > and resume, so hook up the code to restore it. >> > >> > Reported-by: Daniel Vetter <daniel.veter@ffwll.ch> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > Cc: Daniel Vetter <daniel.veter@ffwll.ch> >> >> Testcase: igt/drv_suspend/forcewake? Or have I screwed it up and the test >> is bonkers? > > The testcase relies on the kernel exploding. We could try reading > RING_CTL for a couple of seconds as see if returns 0. Yeah, I somehow thought that we'd underrun the runtime pm refcount, but we only underrun the forcewake refcounts. I guess that's a good justification as any to WARN if the refcount is 0 and we attempt a put. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3fbf8aa8d119..aa03af611ae8 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -544,8 +544,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) int error = 0; intel_uncore_early_sanitize(dev); - - intel_uncore_sanitize(dev); + intel_uncore_restore(dev); if (drm_core_check_feature(dev, DRIVER_MODESET) && restore_gtt_mappings) { @@ -879,6 +878,8 @@ static int i915_runtime_resume(struct device *device) intel_opregion_notify_adapter(dev, PCI_D0); dev_priv->pm.suspended = false; + intel_uncore_restore(dev); + return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e63cfc88c178..c0990eeabd49 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2189,6 +2189,7 @@ extern void intel_uncore_sanitize(struct drm_device *dev); extern void intel_uncore_early_sanitize(struct drm_device *dev); extern void intel_uncore_init(struct drm_device *dev); extern void intel_uncore_check_errors(struct drm_device *dev); +extern void intel_uncore_restore(struct drm_device *dev); extern void intel_uncore_fini(struct drm_device *dev); void diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 34edd7f22f17..e0bf0e54e340 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -831,6 +831,12 @@ void intel_uncore_init(struct drm_device *dev) } } +void intel_uncore_restore(struct drm_device *dev) +{ + intel_uncore_sanitize(dev); + intel_uncore_forcewake_reset(dev, true); +} + void intel_uncore_fini(struct drm_device *dev) { /* Paranoia: make sure we have disabled everything before we exit. */
Daniel stopped that we lose track of the user forcewake across suspend and resume, so hook up the code to restore it. Reported-by: Daniel Vetter <daniel.veter@ffwll.ch> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.veter@ffwll.ch> --- drivers/gpu/drm/i915/i915_drv.c | 5 +++-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_uncore.c | 6 ++++++ 3 files changed, 10 insertions(+), 2 deletions(-)