| Message ID | 1363879819-6943-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 21, 2013 at 03:30:19PM +0000, Chris Wilson wrote: > In order to fully serialize access to the fenced region and the update > to the fence register we need to take extreme measures on SNB+, and > write the fence from each cpu taking care to serialise memory accesses > on each. The usual mb(), or even a mb() on each CPU is not enough to > ensure that access to the fenced region is coherent across the change in > fence register. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/i915/i915_gem.c | 34 ++++++++++++++++++++++++++++++---- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 19fc21b..fe34240 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2684,17 +2684,43 @@ static inline int fence_number(struct drm_i915_private *dev_priv, > return fence - dev_priv->fence_regs; > } > > +struct write_fence { > + struct drm_device *dev; > + struct drm_i915_gem_object *obj; > + int fence; > +}; > + > +static void i915_gem_write_fence__ipi(void *data) > +{ > + struct write_fence *args = data; > + i915_gem_write_fence(args->dev, args->fence, args->obj); > +} > + > static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj, > struct drm_i915_fence_reg *fence, > bool enable) > { > struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > - int reg = fence_number(dev_priv, fence); > - > - i915_gem_write_fence(obj->base.dev, reg, enable ? obj : NULL); > + struct write_fence args = { > + .dev = obj->base.dev, > + .fence = fence_number(dev_priv, fence), > + .obj = enable ? obj : NULL, > + }; > + > + /* In order to fully serialize access to the fenced region and > + * the update to the fence register we need to take extreme > + * measures on SNB+, and write the fence from each cpu taking > + * care to serialise memory accesses on each. The usual mb(), > + * or even a mb() on each CPU is not enough to ensure that access > + * to the fenced region is coherent across the change in fence > + * register. > + */ > + if (!HAS_LLC(obj->base.dev) || > + on_each_cpu(i915_gem_write_fence__ipi, &args, 1) != 0) > + i915_gem_write_fence__ipi(&args); I think the if condition here is a notch to clever and hides the elefantentöter a bit too well. on_each_cpu always calls the given function unconditionally, even when the ipi function call fails, so if (!HAS_LLC) WARN_ON(on_each_cpu); else i915_gem_write_fence looks clearer to me. -Daniel > > if (enable) { > - obj->fence_reg = reg; > + obj->fence_reg = args.fence; > fence->obj = obj; > list_move_tail(&fence->lru_list, &dev_priv->mm.fence_list); > } else { > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 19fc21b..fe34240 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2684,17 +2684,43 @@ static inline int fence_number(struct drm_i915_private *dev_priv, return fence - dev_priv->fence_regs; } +struct write_fence { + struct drm_device *dev; + struct drm_i915_gem_object *obj; + int fence; +}; + +static void i915_gem_write_fence__ipi(void *data) +{ + struct write_fence *args = data; + i915_gem_write_fence(args->dev, args->fence, args->obj); +} + static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj, struct drm_i915_fence_reg *fence, bool enable) { struct drm_i915_private *dev_priv = obj->base.dev->dev_private; - int reg = fence_number(dev_priv, fence); - - i915_gem_write_fence(obj->base.dev, reg, enable ? obj : NULL); + struct write_fence args = { + .dev = obj->base.dev, + .fence = fence_number(dev_priv, fence), + .obj = enable ? obj : NULL, + }; + + /* In order to fully serialize access to the fenced region and + * the update to the fence register we need to take extreme + * measures on SNB+, and write the fence from each cpu taking + * care to serialise memory accesses on each. The usual mb(), + * or even a mb() on each CPU is not enough to ensure that access + * to the fenced region is coherent across the change in fence + * register. + */ + if (!HAS_LLC(obj->base.dev) || + on_each_cpu(i915_gem_write_fence__ipi, &args, 1) != 0) + i915_gem_write_fence__ipi(&args); if (enable) { - obj->fence_reg = reg; + obj->fence_reg = args.fence; fence->obj = obj; list_move_tail(&fence->lru_list, &dev_priv->mm.fence_list); } else {
In order to fully serialize access to the fenced region and the update to the fence register we need to take extreme measures on SNB+, and write the fence from each cpu taking care to serialise memory accesses on each. The usual mb(), or even a mb() on each CPU is not enough to ensure that access to the fenced region is coherent across the change in fence register. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org --- drivers/gpu/drm/i915/i915_gem.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-)