On 19/06/2017 13:22, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2017-06-19 13:06:18)
On 16/06/2017 17:02, Chris Wilson wrote:
@@ -520,7 +511,8 @@ static inline int use_cpu_reloc(const struct reloc_cache *cache,
static int eb_reserve_vma(const struct i915_execbuffer *eb,
struct i915_vma *vma)
{
- struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
+ struct drm_i915_gem_exec_object2 *entry =
+ &eb->exec[vma->exec_flags - eb->flags];
There are three instances of this so maybe "entry = exec_entry(eb,
vma)"? Added benefit that exec_entry implementation could site a comment
explaining the magic.
You were meant to jump in and suggest some way I would store the index.
:)
No ideas I'm afraid, maybe it is just to hot. :)
@@ -870,23 +864,20 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
unsigned int i;
for (i = 0; i < count; i++) {
- struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
- struct i915_vma *vma = exec_to_vma(entry);
+ struct i915_vma *vma = eb->vma[i];
+ unsigned int flags = eb->flags[i];
if (!vma)
continue;
- GEM_BUG_ON(vma->exec_entry != entry);
- vma->exec_entry = NULL;
+ GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
+ vma->exec_flags = NULL;
- if (entry->flags & __EXEC_OBJECT_HAS_PIN)
- __eb_unreserve_vma(vma, entry);
+ if (flags & __EXEC_OBJECT_HAS_PIN)
+ __eb_unreserve_vma(vma, flags);
- if (entry->flags & __EXEC_OBJECT_HAS_REF)
+ if (flags & __EXEC_OBJECT_HAS_REF)
i915_vma_put(vma);
-
- entry->flags &=
- ~(__EXEC_OBJECT_RESERVED | __EXEC_OBJECT_HAS_REF);
No need to clear these from eb->flags[i] to ensure no double free?
No, the other path that may drop the vma prevents the double-free by
setting count to 0. Along the error path we don't have the vma set for
incomplete loads.
Cool.
@@ -1830,33 +1824,34 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
return -ENOMEM;
capture->next = eb->request->capture_list;
- capture->vma = vma;
+ capture->vma = eb->vma[i];
eb->request->capture_list = capture;
}
- if (entry->flags & EXEC_OBJECT_ASYNC)
- goto skip_flushes;
+ if (flags & EXEC_OBJECT_ASYNC)
+ continue;
+ obj = eb->vma[i]->obj;
if (unlikely(obj->cache_dirty && !obj->cache_coherent))
i915_gem_clflush_object(obj, 0);
err = i915_gem_request_await_object
- (eb->request, obj, entry->flags & EXEC_OBJECT_WRITE);
+ (eb->request, obj, flags & EXEC_OBJECT_WRITE);
if (err)
return err;
-
-skip_flushes:
- i915_vma_move_to_active(vma, eb->request, entry->flags);
- __eb_unreserve_vma(vma, entry);
- vma->exec_entry = NULL;
Grumble, it would have been better if this change was in a separate patch.
Grumble. It was more meaningful at some earlier version of the patch
where there was a dance here with flags.
Can't spot any issues anyway:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
