| Message ID | 20170721145037.25105-4-chris@chris-wilson.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series |
"Series without cover letter"
( rev:
1
)
in
Intel GFX |
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index f30675c6bc4a..ae47e1837415 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1776,7 +1776,7 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb) } } - return err ?: have_copy; + return err; } static int eb_relocate(struct i915_execbuffer *eb) @@ -2210,7 +2210,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_rpm; err = eb_relocate(&eb); - if (err) + if (err) { /* * If the user expects the execobject.offset and * reloc.presumed_offset to be an exact match, @@ -2219,8 +2219,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, * relocation. */ args->flags &= ~__EXEC_HAS_RELOC; - if (err < 0) goto err_vma; + } if (unlikely(eb.batch->exec_entry->flags & EXEC_OBJECT_WRITE)) { DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
On pe, 2017-07-21 at 15:50 +0100, Chris Wilson wrote: > I was being overly paranoid in not updating the execobject.offset after > performing the fallback copy where we set reloc.presumed_offset to -1. > The thinking was to ensure that a subsequent NORELOC execbuf would be > forced to process the invalid relocations. However this is overkill so > long as we *only* update the execobject.offset following a successful > update of the relocation value witin the batch. If we have to repeat the > execbuf due to a later interruption, then we may skip the relocations on > the second pass (honouring NORELOC) since the execobject.offset match > the actual offsets (even though reloc.presumed_offset is garbage). > > Subsequent calls to execbuf with NORELOC should themselves ensure that > the reloc.presumed_offset have been corrected in case of future > migration. > > Reporting back the actual execobject.offset, even when > reloc.presumed_offset is garbage, ensures that reuse of those objects > use the latest information to avoid relocations. > > Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array") > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101635 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
I was being overly paranoid in not updating the execobject.offset after performing the fallback copy where we set reloc.presumed_offset to -1. The thinking was to ensure that a subsequent NORELOC execbuf would be forced to process the invalid relocations. However this is overkill so long as we *only* update the execobject.offset following a successful update of the relocation value witin the batch. If we have to repeat the execbuf due to a later interruption, then we may skip the relocations on the second pass (honouring NORELOC) since the execobject.offset match the actual offsets (even though reloc.presumed_offset is garbage). Subsequent calls to execbuf with NORELOC should themselves ensure that the reloc.presumed_offset have been corrected in case of future migration. Reporting back the actual execobject.offset, even when reloc.presumed_offset is garbage, ensures that reuse of those objects use the latest information to avoid relocations. Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101635 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)