On Wed, Jan 04, 2017 at 03:36:16PM +0000, Tvrtko Ursulin wrote: > > On 04/01/2017 14:51, Chris Wilson wrote: > >During the fence registers are clobbered by a GPU reset. Hence if > > During the reset I suppose, although it would sound still a bit > redundant. So maybe "Since GPU reset clobbers the fence registers, > if there is concurrent..." I was just going to leave it as "The fence registers are clobbered by a GPU reset. If..." > >there is concurrent user access to a fenced region via a GTT mmaping, > >the access will not be fenced during the reset (until we restore the > >fences afterwards). In order to prevent invalid access during the reset, > >before we clobber the fences we must invalidate the GTT mmapings. Access > >to the mmap will then be forced to fault the page, and in handling the > >fault, i915_gem_fault() will take the struct_mutex and wait upon the > >reset to complete. > > > >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99274 > >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >--- > > drivers/gpu/drm/i915/i915_drv.c | 3 ++- > > drivers/gpu/drm/i915/i915_drv.h | 4 +++- > > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- > > drivers/gpu/drm/i915/i915_gem_fence_reg.c | 24 ++++++++++++++++++++++++ > > 4 files changed, 35 insertions(+), 3 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > >index 72e29fcb1638..39463fcab593 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.c > >+++ b/drivers/gpu/drm/i915/i915_drv.c > >@@ -1780,6 +1780,7 @@ void i915_reset(struct drm_i915_private *dev_priv) > > error->reset_count++; > > > > pr_notice("drm/i915: Resetting chip after gpu hang\n"); > >+ i915_gem_reset_prepare(dev_priv); > > > > disable_engines_irq(dev_priv); > > ret = intel_gpu_reset(dev_priv, ALL_ENGINES); > >@@ -1793,7 +1794,7 @@ void i915_reset(struct drm_i915_private *dev_priv) > > goto error; > > } > > > >- i915_gem_reset(dev_priv); > >+ i915_gem_reset_finish(dev_priv); > > intel_overlay_reset(dev_priv); > > > > /* Ok, now get things going again... */ > >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >index c035b6576efa..f16986cfa127 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.h > >+++ b/drivers/gpu/drm/i915/i915_drv.h > >@@ -3355,7 +3355,8 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error) > > return READ_ONCE(error->reset_count); > > } > > > >-void i915_gem_reset(struct drm_i915_private *dev_priv); > >+void i915_gem_reset_prepare(struct drm_i915_private *dev_priv); > >+void i915_gem_reset_finish(struct drm_i915_private *dev_priv); > > void i915_gem_set_wedged(struct drm_i915_private *dev_priv); > > > > void i915_gem_clflush_init(struct drm_i915_private *i915); > >@@ -3426,6 +3427,7 @@ i915_vm_to_ppgtt(struct i915_address_space *vm) > > int __must_check i915_vma_get_fence(struct i915_vma *vma); > > int __must_check i915_vma_put_fence(struct i915_vma *vma); > > > >+void i915_gem_revoke_fences(struct drm_i915_private *dev_priv); > > void i915_gem_restore_fences(struct drm_i915_private *dev_priv); > > > > void i915_gem_detect_bit_6_swizzle(struct drm_i915_private *dev_priv); > >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >index 73987014a7bd..019ea7cb13b5 100644 > >--- a/drivers/gpu/drm/i915/i915_gem.c > >+++ b/drivers/gpu/drm/i915/i915_gem.c > >@@ -2715,6 +2715,11 @@ static void reset_request(struct drm_i915_gem_request *request) > > memset(vaddr + head, 0, request->postfix - head); > > } > > > >+void i915_gem_reset_prepare(struct drm_i915_private *dev_priv) > >+{ > >+ i915_gem_revoke_fences(dev_priv); > >+} > >+ > > static void i915_gem_reset_engine(struct intel_engine_cs *engine) > > { > > struct drm_i915_gem_request *request; > >@@ -2780,7 +2785,7 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine) > > spin_unlock_irqrestore(&engine->timeline->lock, flags); > > } > > > >-void i915_gem_reset(struct drm_i915_private *dev_priv) > >+void i915_gem_reset_finish(struct drm_i915_private *dev_priv) > > { > > struct intel_engine_cs *engine; > > enum intel_engine_id id; > >diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c > >index 67835d7b39c7..447abb1105ab 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c > >+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c > >@@ -358,6 +358,30 @@ i915_vma_get_fence(struct i915_vma *vma) > > } > > > > /** > >+ * i915_gem_revoke_fences - revoke fence state > >+ * @dev_priv: i915 device private > >+ * > >+ * Clears the hw fence state and removes all GTT mmappings via the fence. This > > The function doesn't clear the hw fence state - reset does that, no? The first version did, then realised all that was required was zapping the mmap. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
