| 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)
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> --- drivers/gpu/drm/i915/intel_uncore.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)