| Message ID | 1343697866-5572-1-git-send-email-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Eric Anholt <eric@anholt.net> writes: > I have not tested it yet, but hopefully when cworth gets home he will. Reviewed-by: Carl Worth <cworth@cworth.org> Tested-by: Carl Worth <cworth@cworth.org> Thanks, Eric! This is perfect. -Carl
On Mon, 30 Jul 2012 18:24:26 -0700, Eric Anholt <eric@anholt.net> wrote: > If a buffer that was the target of a PIPE_CONTROL from userland was a > reused one that hadn't been evicted which had not previously had this > workaround applied, then we would not bind it into the GTT and the > write would land somewhere else. > > Based on a doubting-my-sanity debugging session with cworth, I'm > pretty sure this will fix his reproducible GL_EXT_timer_query > failures, and hopefully the intermittent OQ issues on snb that > danvet's been working on. > > I have not tested it yet, but hopefully when cworth gets home he will. Had to look hard to find the secret ingredient. The reason why this patch will work is that that in the middle of the error checking we have an early success return after applying the new reloc domains iff the object still holds the same GTT offset as presumed by the reloc. Probably deserves a little more clarification in the changelog as to why the patch works. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> I think this should also fix https://bugs.freedesktop.org/show_bug.cgi?id=48019 but my snb has decided today is the day to hard hang instead. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 25b2c54..afb312e 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -117,6 +117,16 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, target_i915_obj = to_intel_bo(target_obj); target_offset = target_i915_obj->gtt_offset; + /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and + * pipe_control writes because the gpu doesn't properly redirect them + * through the ppgtt for non_secure batchbuffers. */ + if (unlikely(IS_GEN6(dev) && + reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && + !target_i915_obj->has_global_gtt_mapping)) { + i915_gem_gtt_bind_object(target_i915_obj, + target_i915_obj->cache_level); + } + /* The target buffer should have appeared before us in the * exec_object list, so it should have a GTT space bound by now. */ @@ -225,16 +235,6 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, io_mapping_unmap_atomic(reloc_page); } - /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and - * pipe_control writes because the gpu doesn't properly redirect them - * through the ppgtt for non_secure batchbuffers. */ - if (unlikely(IS_GEN6(dev) && - reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && - !target_i915_obj->has_global_gtt_mapping)) { - i915_gem_gtt_bind_object(target_i915_obj, - target_i915_obj->cache_level); - } - /* and update the user's relocation entry */ reloc->presumed_offset = target_offset;
If a buffer that was the target of a PIPE_CONTROL from userland was a reused one that hadn't been evicted which had not previously had this workaround applied, then we would not bind it into the GTT and the write would land somewhere else. Based on a doubting-my-sanity debugging session with cworth, I'm pretty sure this will fix his reproducible GL_EXT_timer_query failures, and hopefully the intermittent OQ issues on snb that danvet's been working on. I have not tested it yet, but hopefully when cworth gets home he will. Signed-off-by: Eric Anholt <eric@anholt.net> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)