Message ID | 1446721898-1450-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Nov 05, 2015 at 01:11:38PM +0200, Mika Kuoppala wrote: > We have a timed release of a forcewake when using > I915_READ/WRITE macros. wait_for() macro will go to quite > long sleep if the first read doesn't satisfy the condition for > successful exit. With these two interacting, it is possible that > we lose the forcewake during the wait_for() and the subsequent read > will reaquire forcewake. > > Further experiments with skl shows that when we lose forcewake, > we lose the reset request we submitted. So reset request register > is not power context saved. > > Grab forcewakes for all engines before starting the reset/request > dance so that all requests stay valid for the duration of reset > requisition across all the engines. > > v2: Add comment on power well sleeps. Wrap the reset handling > under forcewake instead of just reset requests (Chris) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92774 > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Tested-by: Tomi Sarvela <tomix.p.sarvela@intel.com> (v1, v2) > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Thu, 05 Nov 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, Nov 05, 2015 at 01:11:38PM +0200, Mika Kuoppala wrote: >> We have a timed release of a forcewake when using >> I915_READ/WRITE macros. wait_for() macro will go to quite >> long sleep if the first read doesn't satisfy the condition for >> successful exit. With these two interacting, it is possible that >> we lose the forcewake during the wait_for() and the subsequent read >> will reaquire forcewake. >> >> Further experiments with skl shows that when we lose forcewake, >> we lose the reset request we submitted. So reset request register >> is not power context saved. >> >> Grab forcewakes for all engines before starting the reset/request >> dance so that all requests stay valid for the duration of reset >> requisition across all the engines. >> >> v2: Add comment on power well sleeps. Wrap the reset handling >> under forcewake instead of just reset requests (Chris) >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92774 >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Tested-by: Tomi Sarvela <tomix.p.sarvela@intel.com> (v1, v2) >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Pushed to drm-intel-next-fixes, thanks for the patch and review. BR, Jani.
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index f0f97b2..5bb269c 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1530,13 +1530,22 @@ static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) int intel_gpu_reset(struct drm_device *dev) { + struct drm_i915_private *dev_priv = to_i915(dev); int (*reset)(struct drm_device *); + int ret; reset = intel_get_gpu_reset(dev); if (reset == NULL) return -ENODEV; - return reset(dev); + /* If the power well sleeps during the reset, the reset + * request may be dropped and never completes (causing -EIO). + */ + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); + ret = reset(dev); + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); + + return ret; } bool intel_has_gpu_reset(struct drm_device *dev)