| Message ID | 1375315222-4785-25-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 31, 2013 at 05:00:17PM -0700, Ben Widawsky wrote: > In order to transition more of our code over to using a VMA instead of > an <OBJ, VM> pair - we must have the vma accessible at execbuf time. Up > until now, we've only had a VMA when actually binding an object. > > The previous patch helped handle the distinction on bound vs. unbound. > This patch will help us catch leaks, and other issues before we actually > shuffle a bunch of stuff around. > > The subsequent patch to fix up the rest of execbuf should be mostly just > moving code around, and this is the major functional change. > > v2: Release table_lock earlier so vma allocation needn't be atomic. > (Chris) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Merged up to this patch to dinq, thanks. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++++++++------- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 18 +++++++++++++----- > 3 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f6c2812..c0eb7fd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1857,6 +1857,9 @@ unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, > struct i915_address_space *vm); > struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, > struct i915_address_space *vm); > +struct i915_vma * > +i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm); > /* Some GGTT VM helpers */ > #define obj_to_ggtt(obj) \ > (&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base) > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 21331d8..72bd53c 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3101,8 +3101,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > struct i915_vma *vma; > int ret; > > - if (WARN_ON(!list_empty(&obj->vma_list))) > - return -EBUSY; > + BUG_ON(!i915_is_ggtt(vm)); > > fence_size = i915_gem_get_gtt_size(dev, > obj->base.size, > @@ -3142,16 +3141,15 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > > i915_gem_object_pin_pages(obj); > > - /* FIXME: For now we only ever use 1 VMA per object */ > - BUG_ON(!i915_is_ggtt(vm)); > - WARN_ON(!list_empty(&obj->vma_list)); > - > - vma = i915_gem_vma_create(obj, vm); > + vma = i915_gem_obj_lookup_or_create_vma(obj, vm); > if (IS_ERR(vma)) { > i915_gem_object_unpin_pages(obj); > return PTR_ERR(vma); > } > > + /* For now we only ever use 1 vma per object */ > + WARN_ON(!list_is_singular(&obj->vma_list)); > + > search_free: > ret = drm_mm_insert_node_in_range_generic(&vm->mm, &vma->node, > size, alignment, > @@ -4800,3 +4798,16 @@ struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, > > return NULL; > } > + > +struct i915_vma * > +i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm) > +{ > + struct i915_vma *vma; > + > + vma = i915_gem_obj_to_vma(obj, vm); > + if (!vma) > + vma = i915_gem_vma_create(obj, vm); > + > + return vma; > +} > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 0f21702..3f17a55 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -85,14 +85,14 @@ static int > eb_lookup_objects(struct eb_objects *eb, > struct drm_i915_gem_exec_object2 *exec, > const struct drm_i915_gem_execbuffer2 *args, > + struct i915_address_space *vm, > struct drm_file *file) > { > + struct drm_i915_gem_object *obj; > int i; > > spin_lock(&file->table_lock); > for (i = 0; i < args->buffer_count; i++) { > - struct drm_i915_gem_object *obj; > - > obj = to_intel_bo(idr_find(&file->object_idr, exec[i].handle)); > if (obj == NULL) { > spin_unlock(&file->table_lock); > @@ -110,6 +110,15 @@ eb_lookup_objects(struct eb_objects *eb, > > drm_gem_object_reference(&obj->base); > list_add_tail(&obj->exec_list, &eb->objects); > + } > + spin_unlock(&file->table_lock); > + > + list_for_each_entry(obj, &eb->objects, exec_list) { > + struct i915_vma *vma; > + > + vma = i915_gem_obj_lookup_or_create_vma(obj, vm); > + if (IS_ERR(vma)) > + return PTR_ERR(vma); > > obj->exec_entry = &exec[i]; > if (eb->and < 0) { > @@ -121,7 +130,6 @@ eb_lookup_objects(struct eb_objects *eb, > &eb->buckets[handle & eb->and]); > } > } > - spin_unlock(&file->table_lock); > > return 0; > } > @@ -672,7 +680,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, > > /* reacquire the objects */ > eb_reset(eb); > - ret = eb_lookup_objects(eb, exec, args, file); > + ret = eb_lookup_objects(eb, exec, args, vm, file); > if (ret) > goto err; > > @@ -1009,7 +1017,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > } > > /* Look up object handles */ > - ret = eb_lookup_objects(eb, exec, args, file); > + ret = eb_lookup_objects(eb, exec, args, vm, file); > if (ret) > goto err; > > -- > 1.8.3.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_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f6c2812..c0eb7fd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1857,6 +1857,9 @@ unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, struct i915_address_space *vm); struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, struct i915_address_space *vm); +struct i915_vma * +i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, + struct i915_address_space *vm); /* Some GGTT VM helpers */ #define obj_to_ggtt(obj) \ (&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 21331d8..72bd53c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3101,8 +3101,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, struct i915_vma *vma; int ret; - if (WARN_ON(!list_empty(&obj->vma_list))) - return -EBUSY; + BUG_ON(!i915_is_ggtt(vm)); fence_size = i915_gem_get_gtt_size(dev, obj->base.size, @@ -3142,16 +3141,15 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, i915_gem_object_pin_pages(obj); - /* FIXME: For now we only ever use 1 VMA per object */ - BUG_ON(!i915_is_ggtt(vm)); - WARN_ON(!list_empty(&obj->vma_list)); - - vma = i915_gem_vma_create(obj, vm); + vma = i915_gem_obj_lookup_or_create_vma(obj, vm); if (IS_ERR(vma)) { i915_gem_object_unpin_pages(obj); return PTR_ERR(vma); } + /* For now we only ever use 1 vma per object */ + WARN_ON(!list_is_singular(&obj->vma_list)); + search_free: ret = drm_mm_insert_node_in_range_generic(&vm->mm, &vma->node, size, alignment, @@ -4800,3 +4798,16 @@ struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, return NULL; } + +struct i915_vma * +i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, + struct i915_address_space *vm) +{ + struct i915_vma *vma; + + vma = i915_gem_obj_to_vma(obj, vm); + if (!vma) + vma = i915_gem_vma_create(obj, vm); + + return vma; +} diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0f21702..3f17a55 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -85,14 +85,14 @@ static int eb_lookup_objects(struct eb_objects *eb, struct drm_i915_gem_exec_object2 *exec, const struct drm_i915_gem_execbuffer2 *args, + struct i915_address_space *vm, struct drm_file *file) { + struct drm_i915_gem_object *obj; int i; spin_lock(&file->table_lock); for (i = 0; i < args->buffer_count; i++) { - struct drm_i915_gem_object *obj; - obj = to_intel_bo(idr_find(&file->object_idr, exec[i].handle)); if (obj == NULL) { spin_unlock(&file->table_lock); @@ -110,6 +110,15 @@ eb_lookup_objects(struct eb_objects *eb, drm_gem_object_reference(&obj->base); list_add_tail(&obj->exec_list, &eb->objects); + } + spin_unlock(&file->table_lock); + + list_for_each_entry(obj, &eb->objects, exec_list) { + struct i915_vma *vma; + + vma = i915_gem_obj_lookup_or_create_vma(obj, vm); + if (IS_ERR(vma)) + return PTR_ERR(vma); obj->exec_entry = &exec[i]; if (eb->and < 0) { @@ -121,7 +130,6 @@ eb_lookup_objects(struct eb_objects *eb, &eb->buckets[handle & eb->and]); } } - spin_unlock(&file->table_lock); return 0; } @@ -672,7 +680,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, /* reacquire the objects */ eb_reset(eb); - ret = eb_lookup_objects(eb, exec, args, file); + ret = eb_lookup_objects(eb, exec, args, vm, file); if (ret) goto err; @@ -1009,7 +1017,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } /* Look up object handles */ - ret = eb_lookup_objects(eb, exec, args, file); + ret = eb_lookup_objects(eb, exec, args, vm, file); if (ret) goto err;
In order to transition more of our code over to using a VMA instead of an <OBJ, VM> pair - we must have the vma accessible at execbuf time. Up until now, we've only had a VMA when actually binding an object. The previous patch helped handle the distinction on bound vs. unbound. This patch will help us catch leaks, and other issues before we actually shuffle a bunch of stuff around. The subsequent patch to fix up the rest of execbuf should be mostly just moving code around, and this is the major functional change. v2: Release table_lock earlier so vma allocation needn't be atomic. (Chris) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++++++++------- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 18 +++++++++++++----- 3 files changed, 34 insertions(+), 12 deletions(-)