| Message ID | 1302640318-23165-28-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 12, 2011 at 09:31:55PM +0100, Chris Wilson wrote: > Whenever we finish reading an object through a fence, for safety we > clear any GPU read domain and so invalidate any TLBs associated with > the fenced region upon its next use. Now that flush_fence ensures that we are paranoid and flush/invalidate caches the if (fenced_gpu_access && !pending_fenced_gpu_access) flush_some_more(); code is indeed superfluous. But please explain that in the changelog, it has taken me a while to (re-)figure out while the second hunk is correct. Otherwise > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> still stands.
On Wed, 13 Apr 2011 21:43:59 +0200, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Apr 12, 2011 at 09:31:55PM +0100, Chris Wilson wrote: > > Whenever we finish reading an object through a fence, for safety we > > clear any GPU read domain and so invalidate any TLBs associated with > > the fenced region upon its next use. > > Now that flush_fence ensures that we are paranoid and flush/invalidate > caches the > if (fenced_gpu_access && !pending_fenced_gpu_access) > flush_some_more(); > code is indeed superfluous. But please explain that in the changelog, it has > taken me a while to (re-)figure out while the second hunk is correct. Ok: Whenever we finish reading an object through a fence, for safety we clear any GPU read domain and so invalidate any TLBs associated with the fenced region upon its next use. As we now always flush writes through an existing fence before it is released and then trigger the invalidation of the GPU domains should we ever re-use it again on the GPU, we no longer need to compare and force the invalidation if the fenced access changes in move_to_gpu(). -Chris
On Wed, Apr 13, 2011 at 09:38:50PM +0100, Chris Wilson wrote: > Whenever we finish reading an object through a fence, for safety we > clear any GPU read domain and so invalidate any TLBs associated with > the fenced region upon its next use. As we now always flush writes > through an existing fence before it is released and then trigger the > invalidation of the GPU domains should we ever re-use it again on the > GPU, we no longer need to compare and force the invalidation if the > fenced access changes in move_to_gpu(). Sounds good. And now I need some sleep, so the next mail will take longer to answer ;-) Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0f9d007..ad0c2b7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2508,6 +2508,8 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj, return ret; } + /* Invalidate the GPU TLBs for any future reads */ + obj->base.read_domains &= ~I915_GEM_GPU_DOMAINS; obj->fenced_gpu_access = false; } diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a07911f..0010aee 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -172,9 +172,8 @@ i915_gem_object_set_to_gpu_domain(struct drm_i915_gem_object *obj, * write domain */ if (obj->base.write_domain && - (((obj->base.write_domain != obj->base.pending_read_domains || - obj->ring != ring)) || - (obj->fenced_gpu_access && !obj->pending_fenced_gpu_access))) { + (obj->base.write_domain != obj->base.pending_read_domains || + obj->ring != ring)) { flush_domains |= obj->base.write_domain; invalidate_domains |= obj->base.pending_read_domains & ~obj->base.write_domain;