[Intel-gfx] [PATCH 04/13] drm/i915: Create/destroy VM (ppGTT) for use with contexts
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri Mar 8 15:03:15 UTC 2019
On 08/03/2019 14:12, Chris Wilson wrote:
> In preparation to making the ppGTT binding for a context explicit (to
> facilitate reusing the same ppGTT between different contexts), allow the
> user to create and destroy named ppGTT.
>
> v2: Replace global barrier for swapping over the ppgtt and tlbs with a
> local context barrier (Tvrtko)
> v3: serialise with struct_mutex; it's lazy but required dammit
> v4: Rewrite igt_ctx_shared_exec to be more different (aimed to be more
> similarly, turned out different!)
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 +
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/i915_gem_context.c | 254 +++++++++++++++++-
> drivers/gpu/drm/i915/i915_gem_context.h | 5 +
> drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +-
> drivers/gpu/drm/i915/i915_gem_gtt.h | 16 +-
> drivers/gpu/drm/i915/selftests/huge_pages.c | 1 -
> .../gpu/drm/i915/selftests/i915_gem_context.c | 215 ++++++++++++---
> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 1 -
> drivers/gpu/drm/i915/selftests/mock_context.c | 8 +-
> include/uapi/drm/i915_drm.h | 36 +++
> 11 files changed, 497 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0d743907e7bc..5d53efc4c5d9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -3121,6 +3121,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_GEM_VM_CREATE, i915_gem_vm_create_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_GEM_VM_DESTROY, i915_gem_vm_destroy_ioctl, DRM_RENDER_ALLOW),
> };
>
> static struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c4ffe19ec698..8c4eb302cc0b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -218,6 +218,9 @@ struct drm_i915_file_private {
> } mm;
> struct idr context_idr;
>
> + struct mutex vm_lock;
> + struct idr vm_idr;
> +
> unsigned int bsd_engine;
>
> /*
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index b6370225dcb5..fb2aba06f693 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -126,6 +126,8 @@ static void lut_close(struct i915_gem_context *ctx)
> struct i915_vma *vma = rcu_dereference_raw(*slot);
>
> radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);
> +
> + vma->open_count--;
Okay figured it out in N-th attempt.. I think.. lut gets unlinked from
the obj lut list before this decrement so there can not be a double
decrement.
> __i915_gem_object_release_unless_active(vma->obj);
> }
> rcu_read_unlock();
> @@ -306,7 +308,7 @@ static void context_close(struct i915_gem_context *ctx)
> */
> lut_close(ctx);
> if (ctx->ppgtt)
> - i915_ppgtt_close(&ctx->ppgtt->vm);
> + i915_ppgtt_close(ctx->ppgtt);
>
> ctx->file_priv = ERR_PTR(-EBADF);
> i915_gem_context_put(ctx);
> @@ -417,6 +419,32 @@ static void __destroy_hw_context(struct i915_gem_context *ctx,
> context_close(ctx);
> }
>
> +static struct i915_hw_ppgtt *
> +__set_ppgtt(struct i915_gem_context *ctx, struct i915_hw_ppgtt *ppgtt)
> +{
> + struct i915_hw_ppgtt *old = ctx->ppgtt;
> +
> + i915_ppgtt_open(ppgtt);
> + ctx->ppgtt = i915_ppgtt_get(ppgtt);
> +
> + ctx->desc_template = default_desc_template(ctx->i915, ppgtt);
> +
> + return old;
> +}
> +
> +static void __assign_ppgtt(struct i915_gem_context *ctx,
> + struct i915_hw_ppgtt *ppgtt)
> +{
> + if (ppgtt == ctx->ppgtt)
> + return;
> +
> + ppgtt = __set_ppgtt(ctx, ppgtt);
> + if (ppgtt) {
> + i915_ppgtt_close(ppgtt);
> + i915_ppgtt_put(ppgtt);
> + }
> +}
> +
> static struct i915_gem_context *
> i915_gem_create_context(struct drm_i915_private *dev_priv,
> struct drm_i915_file_private *file_priv)
> @@ -443,8 +471,8 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
> return ERR_CAST(ppgtt);
> }
>
> - ctx->ppgtt = ppgtt;
> - ctx->desc_template = default_desc_template(dev_priv, ppgtt);
> + __assign_ppgtt(ctx, ppgtt);
> + i915_ppgtt_put(ppgtt);
> }
>
> trace_i915_context_create(ctx);
> @@ -625,19 +653,29 @@ static int context_idr_cleanup(int id, void *p, void *data)
> return 0;
> }
>
> +static int vm_idr_cleanup(int id, void *p, void *data)
> +{
> + i915_ppgtt_put(p);
> + return 0;
> +}
> +
> int i915_gem_context_open(struct drm_i915_private *i915,
> struct drm_file *file)
> {
> struct drm_i915_file_private *file_priv = file->driver_priv;
> struct i915_gem_context *ctx;
>
> + mutex_init(&file_priv->vm_lock);
> +
> idr_init(&file_priv->context_idr);
> + idr_init_base(&file_priv->vm_idr, 1);
>
> mutex_lock(&i915->drm.struct_mutex);
> ctx = i915_gem_create_context(i915, file_priv);
> mutex_unlock(&i915->drm.struct_mutex);
> if (IS_ERR(ctx)) {
> idr_destroy(&file_priv->context_idr);
> + idr_destroy(&file_priv->vm_idr);
> return PTR_ERR(ctx);
> }
>
> @@ -654,6 +692,89 @@ void i915_gem_context_close(struct drm_file *file)
>
> idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
> idr_destroy(&file_priv->context_idr);
> +
> + idr_for_each(&file_priv->vm_idr, vm_idr_cleanup, NULL);
> + idr_destroy(&file_priv->vm_idr);
> +
> + mutex_destroy(&file_priv->vm_lock);
> +}
> +
> +int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
> +{
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct drm_i915_gem_vm_control *args = data;
> + struct drm_i915_file_private *file_priv = file->driver_priv;
> + struct i915_hw_ppgtt *ppgtt;
> + int err;
> +
> + if (!HAS_FULL_PPGTT(i915))
> + return -ENODEV;
> +
> + if (args->flags)
> + return -EINVAL;
> +
> + if (args->extensions)
> + return -EINVAL;
> +
> + ppgtt = i915_ppgtt_create(i915, file_priv);
> + if (IS_ERR(ppgtt))
> + return PTR_ERR(ppgtt);
> +
> + err = mutex_lock_interruptible(&file_priv->vm_lock);
> + if (err)
> + goto err_put;
> +
> + err = idr_alloc(&file_priv->vm_idr, ppgtt, 0, 0, GFP_KERNEL);
> + mutex_unlock(&file_priv->vm_lock);
> + if (err < 0)
> + goto err_put;
> +
> + GEM_BUG_ON(err == 0); /* reserved for default/unassigned ppgtt */
> + ppgtt->user_handle = err;
> + args->id = err;
> + return 0;
> +
> +err_put:
> + i915_ppgtt_put(ppgtt);
> + return err;
> +}
> +
> +int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
> +{
> + struct drm_i915_file_private *file_priv = file->driver_priv;
> + struct drm_i915_gem_vm_control *args = data;
> + struct i915_hw_ppgtt *ppgtt;
> + int err;
> + u32 id;
> +
> + if (args->flags)
> + return -EINVAL;
> +
> + if (args->extensions)
> + return -EINVAL;
> +
> + id = args->id;
> + if (!id)
> + return -ENOENT;
> +
> + err = mutex_lock_interruptible(&file_priv->vm_lock);
> + if (err)
> + return err;
> +
> + ppgtt = idr_remove(&file_priv->vm_idr, id);
> + if (ppgtt) {
> + GEM_BUG_ON(!ppgtt->user_handle);
> + ppgtt->user_handle = 0;
> + }
> +
> + mutex_unlock(&file_priv->vm_lock);
> + if (!ppgtt)
> + return -ENOENT;
> +
> + i915_ppgtt_put(ppgtt);
> + return 0;
> }
>
> static struct i915_request *
> @@ -799,6 +920,120 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915,
> return 0;
> }
>
> +static int get_ppgtt(struct i915_gem_context *ctx,
> + struct drm_i915_gem_context_param *args)
> +{
> + struct drm_i915_file_private *file_priv = ctx->file_priv;
> + struct i915_hw_ppgtt *ppgtt;
> + int ret;
> +
> + if (!ctx->ppgtt)
> + return -ENODEV;
> +
> + /* XXX rcu acquire? */
> + ret = mutex_lock_interruptible(&ctx->i915->drm.struct_mutex);
> + if (ret)
> + return ret;
> +
> + ppgtt = i915_ppgtt_get(ctx->ppgtt);
> + mutex_unlock(&ctx->i915->drm.struct_mutex);
> +
> + ret = mutex_lock_interruptible(&file_priv->vm_lock);
> + if (ret)
> + goto err_put;
> +
> + if (!ppgtt->user_handle) {
> + ret = idr_alloc(&file_priv->vm_idr, ppgtt, 0, 0, GFP_KERNEL);
> + GEM_BUG_ON(!ret);
> + if (ret < 0)
> + goto err_unlock;
> +
> + ppgtt->user_handle = ret;
> + i915_ppgtt_get(ppgtt);
> + }
> +
> + args->size = 0;
> + args->value = ppgtt->user_handle;
> +
> + ret = 0;
> +err_unlock:
> + mutex_unlock(&file_priv->vm_lock);
> +err_put:
> + i915_ppgtt_put(ppgtt);
> + return ret;
> +}
> +
> +static void set_ppgtt_barrier(void *data)
> +{
> + struct i915_hw_ppgtt *old = data;
> +
> + i915_ppgtt_close(old);
> + i915_ppgtt_put(old);
> +}
> +
> +static int set_ppgtt(struct i915_gem_context *ctx,
> + struct drm_i915_gem_context_param *args)
> +{
> + struct drm_i915_file_private *file_priv = ctx->file_priv;
> + struct i915_hw_ppgtt *ppgtt, *old;
> + int err;
> +
> + if (args->size)
> + return -EINVAL;
> +
> + if (upper_32_bits(args->value))
> + return -EINVAL;
> +
> + if (!ctx->ppgtt)
> + return -ENODEV;
> +
> + err = mutex_lock_interruptible(&file_priv->vm_lock);
> + if (err)
> + return err;
> +
> + ppgtt = idr_find(&file_priv->vm_idr, args->value);
> + if (ppgtt) {
> + GEM_BUG_ON(ppgtt->user_handle != args->value);
> + i915_ppgtt_get(ppgtt);
> + }
> + mutex_unlock(&file_priv->vm_lock);
> + if (!ppgtt)
> + return -ENOENT;
> +
> + err = mutex_lock_interruptible(&ctx->i915->drm.struct_mutex);
> + if (err)
> + goto out;
> +
> + if (ppgtt == ctx->ppgtt)
> + goto unlock;
> +
> + /* Teardown the existing obj:vma cache, it will have to be rebuilt. */
> + lut_close(ctx);
> +
> + old = __set_ppgtt(ctx, ppgtt);
> +
> + /*
> + * We need to flush any requests using the current ppgtt before
> + * we release it as the requests do not hold a reference themselves,
> + * only indirectly through the context.
> + */
> + err = context_barrier_task(ctx, ALL_ENGINES, set_ppgtt_barrier, old);
> + if (err) {
> + ctx->ppgtt = old;
> + ctx->desc_template = default_desc_template(ctx->i915, old);
> +
> + i915_ppgtt_close(ppgtt);
> + i915_ppgtt_put(ppgtt);
> + }
> +
> +unlock:
> + mutex_unlock(&ctx->i915->drm.struct_mutex);
> +
> +out:
> + i915_ppgtt_put(ppgtt);
> + return err;
> +}
> +
> static bool client_is_banned(struct drm_i915_file_private *file_priv)
> {
> return atomic_read(&file_priv->ban_score) >= I915_CLIENT_SCORE_BANNED;
> @@ -973,6 +1208,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> case I915_CONTEXT_PARAM_SSEU:
> ret = get_sseu(ctx, args);
> break;
> + case I915_CONTEXT_PARAM_VM:
> + ret = get_ppgtt(ctx, args);
> + break;
> default:
> ret = -EINVAL;
> break;
> @@ -1274,9 +1512,6 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> return -ENOENT;
>
> switch (args->param) {
> - case I915_CONTEXT_PARAM_BAN_PERIOD:
> - ret = -EINVAL;
> - break;
> case I915_CONTEXT_PARAM_NO_ZEROMAP:
> if (args->size)
> ret = -EINVAL;
> @@ -1332,9 +1567,16 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> I915_USER_PRIORITY(priority);
> }
> break;
> +
> case I915_CONTEXT_PARAM_SSEU:
> ret = set_sseu(ctx, args);
> break;
> +
> + case I915_CONTEXT_PARAM_VM:
> + ret = set_ppgtt(ctx, args);
> + break;
> +
> + case I915_CONTEXT_PARAM_BAN_PERIOD:
> default:
> ret = -EINVAL;
> break;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 5a32c4b4816f..1e670372892c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -153,6 +153,11 @@ void i915_gem_context_release(struct kref *ctx_ref);
> struct i915_gem_context *
> i915_gem_context_create_gvt(struct drm_device *dev);
>
> +int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file);
> +int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file);
> +
> int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index dac08d9c3fab..d717952cc430 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2099,10 +2099,21 @@ i915_ppgtt_create(struct drm_i915_private *i915,
> return ppgtt;
> }
>
> -void i915_ppgtt_close(struct i915_address_space *vm)
> +void i915_ppgtt_open(struct i915_hw_ppgtt *ppgtt)
> {
> - GEM_BUG_ON(vm->closed);
> - vm->closed = true;
> + GEM_BUG_ON(ppgtt->vm.closed);
> +
> + ppgtt->open_count++;
> +}
> +
> +void i915_ppgtt_close(struct i915_hw_ppgtt *ppgtt)
> +{
> + GEM_BUG_ON(!ppgtt->open_count);
> + if (--ppgtt->open_count)
> + return;
> +
> + GEM_BUG_ON(ppgtt->vm.closed);
> + ppgtt->vm.closed = true;
> }
>
> static void ppgtt_destroy_vma(struct i915_address_space *vm)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index a47e11e6fc1b..25d5f7682bda 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -391,11 +391,15 @@ struct i915_hw_ppgtt {
> struct kref ref;
>
> unsigned long pd_dirty_engines;
> + unsigned int open_count;
> +
> union {
> struct i915_pml4 pml4; /* GEN8+ & 48b PPGTT */
> struct i915_page_directory_pointer pdp; /* GEN8+ */
> struct i915_page_directory pd; /* GEN6-7 */
> };
> +
> + u32 user_handle;
> };
>
> struct gen6_hw_ppgtt {
> @@ -606,12 +610,16 @@ int i915_ppgtt_init_hw(struct drm_i915_private *dev_priv);
> void i915_ppgtt_release(struct kref *kref);
> struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_i915_private *dev_priv,
> struct drm_i915_file_private *fpriv);
> -void i915_ppgtt_close(struct i915_address_space *vm);
> -static inline void i915_ppgtt_get(struct i915_hw_ppgtt *ppgtt)
> +
> +void i915_ppgtt_open(struct i915_hw_ppgtt *ppgtt);
> +void i915_ppgtt_close(struct i915_hw_ppgtt *ppgtt);
> +
> +static inline struct i915_hw_ppgtt *i915_ppgtt_get(struct i915_hw_ppgtt *ppgtt)
> {
> - if (ppgtt)
> - kref_get(&ppgtt->ref);
> + kref_get(&ppgtt->ref);
> + return ppgtt;
> }
> +
> static inline void i915_ppgtt_put(struct i915_hw_ppgtt *ppgtt)
> {
> if (ppgtt)
> diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
> index 1e66cff985f8..0b7740dc18cb 100644
> --- a/drivers/gpu/drm/i915/selftests/huge_pages.c
> +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
> @@ -1734,7 +1734,6 @@ int i915_gem_huge_page_mock_selftests(void)
> err = i915_subtests(tests, ppgtt);
>
> out_close:
> - i915_ppgtt_close(&ppgtt->vm);
> i915_ppgtt_put(ppgtt);
>
> out_unlock:
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 664ae1428ecc..c4a5cf26992e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -372,7 +372,8 @@ static int cpu_fill(struct drm_i915_gem_object *obj, u32 value)
> return 0;
> }
>
> -static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
> +static noinline int cpu_check(struct drm_i915_gem_object *obj,
> + unsigned int idx, unsigned int max)
> {
> unsigned int n, m, needs_flush;
> int err;
> @@ -390,8 +391,10 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
>
> for (m = 0; m < max; m++) {
> if (map[m] != m) {
> - pr_err("Invalid value at page %d, offset %d: found %x expected %x\n",
> - n, m, map[m], m);
> + pr_err("%pS: Invalid value at object %d page %d/%ld, offset %d/%d: found %x expected %x\n",
> + __builtin_return_address(0), idx,
> + n, real_page_count(obj), m, max,
> + map[m], m);
> err = -EINVAL;
> goto out_unmap;
> }
> @@ -399,8 +402,9 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
>
> for (; m < DW_PER_PAGE; m++) {
> if (map[m] != STACK_MAGIC) {
> - pr_err("Invalid value at page %d, offset %d: found %x expected %x\n",
> - n, m, map[m], STACK_MAGIC);
> + pr_err("%pS: Invalid value at object %d page %d, offset %d: found %x expected %x (uninitialised)\n",
> + __builtin_return_address(0), idx, n, m,
> + map[m], STACK_MAGIC);
> err = -EINVAL;
> goto out_unmap;
> }
> @@ -478,12 +482,8 @@ static unsigned long max_dwords(struct drm_i915_gem_object *obj)
> static int igt_ctx_exec(void *arg)
> {
> struct drm_i915_private *i915 = arg;
> - struct drm_i915_gem_object *obj = NULL;
> - unsigned long ncontexts, ndwords, dw;
> - struct igt_live_test t;
> - struct drm_file *file;
> - IGT_TIMEOUT(end_time);
> - LIST_HEAD(objects);
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> int err = -ENODEV;
>
> /*
> @@ -495,41 +495,166 @@ static int igt_ctx_exec(void *arg)
> if (!DRIVER_CAPS(i915)->has_logical_contexts)
> return 0;
>
> + for_each_engine(engine, i915, id) {
> + struct drm_i915_gem_object *obj = NULL;
> + unsigned long ncontexts, ndwords, dw;
> + struct igt_live_test t;
> + struct drm_file *file;
> + IGT_TIMEOUT(end_time);
> + LIST_HEAD(objects);
> +
> + if (!intel_engine_can_store_dword(engine))
> + continue;
> +
> + if (!engine->context_size)
> + continue; /* No logical context support in HW */
> +
> + file = mock_file(i915);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
> +
> + mutex_lock(&i915->drm.struct_mutex);
> +
> + err = igt_live_test_begin(&t, i915, __func__, engine->name);
> + if (err)
> + goto out_unlock;
> +
> + ncontexts = 0;
> + ndwords = 0;
> + dw = 0;
> + while (!time_after(jiffies, end_time)) {
> + struct i915_gem_context *ctx;
> + intel_wakeref_t wakeref;
> +
> + ctx = i915_gem_create_context(i915, file->driver_priv);
> + if (IS_ERR(ctx)) {
> + err = PTR_ERR(ctx);
> + goto out_unlock;
> + }
> +
> + if (!obj) {
> + obj = create_test_object(ctx, file, &objects);
> + if (IS_ERR(obj)) {
> + err = PTR_ERR(obj);
> + goto out_unlock;
> + }
> + }
> +
> + with_intel_runtime_pm(i915, wakeref)
> + err = gpu_fill(obj, ctx, engine, dw);
> + if (err) {
> + pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) in ctx %u [full-ppgtt? %s], err=%d\n",
> + ndwords, dw, max_dwords(obj),
> + engine->name, ctx->hw_id,
> + yesno(!!ctx->ppgtt), err);
> + goto out_unlock;
> + }
> +
> + if (++dw == max_dwords(obj)) {
> + obj = NULL;
> + dw = 0;
> + }
> +
> + ndwords++;
> + ncontexts++;
> + }
> +
> + pr_info("Submitted %lu contexts to %s, filling %lu dwords\n",
> + ncontexts, engine->name, ndwords);
> +
> + ncontexts = dw = 0;
> + list_for_each_entry(obj, &objects, st_link) {
> + unsigned int rem =
> + min_t(unsigned int, ndwords - dw, max_dwords(obj));
> +
> + err = cpu_check(obj, ncontexts++, rem);
> + if (err)
> + break;
> +
> + dw += rem;
> + }
> +
> +out_unlock:
> + if (igt_live_test_end(&t))
> + err = -EIO;
> + mutex_unlock(&i915->drm.struct_mutex);
> +
> + mock_file_free(i915, file);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int igt_shared_ctx_exec(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct i915_gem_context *parent;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + struct igt_live_test t;
> + struct drm_file *file;
> + int err = 0;
> +
> + /*
> + * Create a few different contexts with the same mm and write
> + * through each ctx using the GPU making sure those writes end
> + * up in the expected pages of our obj.
> + */
> + if (!DRIVER_CAPS(i915)->has_logical_contexts)
> + return 0;
> +
> file = mock_file(i915);
> if (IS_ERR(file))
> return PTR_ERR(file);
>
> mutex_lock(&i915->drm.struct_mutex);
>
> + parent = i915_gem_create_context(i915, file->driver_priv);
> + if (IS_ERR(parent)) {
> + err = PTR_ERR(parent);
> + goto out_unlock;
> + }
> +
> + if (!parent->ppgtt) {
> + err = -ENODEV;
> + goto out_unlock;
> + }
> +
> err = igt_live_test_begin(&t, i915, __func__, "");
> if (err)
> goto out_unlock;
>
> - ncontexts = 0;
> - ndwords = 0;
> - dw = 0;
> - while (!time_after(jiffies, end_time)) {
> - struct intel_engine_cs *engine;
> - struct i915_gem_context *ctx;
> - unsigned int id;
> + for_each_engine(engine, i915, id) {
> + unsigned long ncontexts, ndwords, dw;
> + struct drm_i915_gem_object *obj = NULL;
> + struct i915_gem_context *ctx = NULL;
> + IGT_TIMEOUT(end_time);
> + LIST_HEAD(objects);
>
> - ctx = i915_gem_create_context(i915, file->driver_priv);
> - if (IS_ERR(ctx)) {
> - err = PTR_ERR(ctx);
> - goto out_unlock;
> - }
> + if (!intel_engine_can_store_dword(engine))
> + continue;
>
> - for_each_engine(engine, i915, id) {
> + dw = 0;
> + ndwords = 0;
> + ncontexts = 0;
> + while (!time_after(jiffies, end_time)) {
> intel_wakeref_t wakeref;
>
> - if (!engine->context_size)
> - continue; /* No logical context support in HW */
> + if (ctx)
> + __destroy_hw_context(ctx, file->driver_priv);
>
> - if (!intel_engine_can_store_dword(engine))
> - continue;
> + ctx = i915_gem_create_context(i915, file->driver_priv);
> + if (IS_ERR(ctx)) {
> + err = PTR_ERR(ctx);
> + goto out_unlock;
> + }
> +
> + __assign_ppgtt(ctx, parent->ppgtt);
>
> if (!obj) {
> - obj = create_test_object(ctx, file, &objects);
> + obj = create_test_object(parent, file, &objects);
> if (IS_ERR(obj)) {
> err = PTR_ERR(obj);
> goto out_unlock;
> @@ -551,25 +676,25 @@ static int igt_ctx_exec(void *arg)
> obj = NULL;
> dw = 0;
> }
> +
> ndwords++;
> + ncontexts++;
> }
> - ncontexts++;
> - }
> - pr_info("Submitted %lu contexts (across %u engines), filling %lu dwords\n",
> - ncontexts, RUNTIME_INFO(i915)->num_engines, ndwords);
> + pr_info("Submitted %lu contexts to %s, filling %lu dwords\n",
> + ncontexts, engine->name, ndwords);
>
> - dw = 0;
> - list_for_each_entry(obj, &objects, st_link) {
> - unsigned int rem =
> - min_t(unsigned int, ndwords - dw, max_dwords(obj));
> + ncontexts = dw = 0;
> + list_for_each_entry(obj, &objects, st_link) {
> + unsigned int rem =
> + min_t(unsigned int, ndwords - dw, max_dwords(obj));
>
> - err = cpu_check(obj, rem);
> - if (err)
> - break;
> + err = cpu_check(obj, ncontexts++, rem);
> + if (err)
> + goto out_unlock;
>
> - dw += rem;
> + dw += rem;
> + }
> }
> -
> out_unlock:
> if (igt_live_test_end(&t))
> err = -EIO;
> @@ -1048,7 +1173,7 @@ static int igt_ctx_readonly(void *arg)
> struct drm_i915_gem_object *obj = NULL;
> struct i915_gem_context *ctx;
> struct i915_hw_ppgtt *ppgtt;
> - unsigned long ndwords, dw;
> + unsigned long idx, ndwords, dw;
> struct igt_live_test t;
> struct drm_file *file;
> I915_RND_STATE(prng);
> @@ -1129,6 +1254,7 @@ static int igt_ctx_readonly(void *arg)
> ndwords, RUNTIME_INFO(i915)->num_engines);
>
> dw = 0;
> + idx = 0;
> list_for_each_entry(obj, &objects, st_link) {
> unsigned int rem =
> min_t(unsigned int, ndwords - dw, max_dwords(obj));
> @@ -1138,7 +1264,7 @@ static int igt_ctx_readonly(void *arg)
> if (i915_gem_object_is_readonly(obj))
> num_writes = 0;
>
> - err = cpu_check(obj, num_writes);
> + err = cpu_check(obj, idx++, num_writes);
> if (err)
> break;
>
> @@ -1723,6 +1849,7 @@ int i915_gem_context_live_selftests(struct drm_i915_private *dev_priv)
> SUBTEST(igt_ctx_exec),
> SUBTEST(igt_ctx_readonly),
> SUBTEST(igt_ctx_sseu),
> + SUBTEST(igt_shared_ctx_exec),
> SUBTEST(igt_vm_isolation),
> };
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 826fd51c331e..57b3d9867070 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -1020,7 +1020,6 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
>
> err = func(dev_priv, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
>
> - i915_ppgtt_close(&ppgtt->vm);
> i915_ppgtt_put(ppgtt);
> out_unlock:
> mutex_unlock(&dev_priv->drm.struct_mutex);
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index 8efa6892c6cd..f90328b21763 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -54,13 +54,17 @@ mock_context(struct drm_i915_private *i915,
> goto err_handles;
>
> if (name) {
> + struct i915_hw_ppgtt *ppgtt;
> +
> ctx->name = kstrdup(name, GFP_KERNEL);
> if (!ctx->name)
> goto err_put;
>
> - ctx->ppgtt = mock_ppgtt(i915, name);
> - if (!ctx->ppgtt)
> + ppgtt = mock_ppgtt(i915, name);
> + if (!ppgtt)
> goto err_put;
> +
> + __set_ppgtt(ctx, ppgtt);
> }
>
> return ctx;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 39835793722b..6575470755d0 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -341,6 +341,8 @@ typedef struct _drm_i915_sarea {
> #define DRM_I915_PERF_ADD_CONFIG 0x37
> #define DRM_I915_PERF_REMOVE_CONFIG 0x38
> #define DRM_I915_QUERY 0x39
> +#define DRM_I915_GEM_VM_CREATE 0x3a
> +#define DRM_I915_GEM_VM_DESTROY 0x3b
> /* Must be kept compact -- no holes */
>
> #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
> @@ -400,6 +402,8 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
> #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64)
> #define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
> +#define DRM_IOCTL_I915_GEM_VM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_CREATE, struct drm_i915_gem_vm_control)
> +#define DRM_IOCTL_I915_GEM_VM_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_VM_DESTROY, struct drm_i915_gem_vm_control)
>
> /* Allow drivers to submit batchbuffers directly to hardware, relying
> * on the security mechanisms provided by hardware.
> @@ -1451,6 +1455,26 @@ struct drm_i915_gem_context_destroy {
> __u32 pad;
> };
>
> +/*
> + * DRM_I915_GEM_VM_CREATE -
> + *
> + * Create a new virtual memory address space (ppGTT) for use within a context
> + * on the same file. Extensions can be provided to configure exactly how the
> + * address space is setup upon creation.
> + *
> + * The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
> + * returned.
returned and store in id - do we need both? Maybe return zero or error
to make it simpler?
> + *
> + * DRM_I915_GEM_VM_DESTROY -
> + *
> + * Destroys a previously created VM id.
> + */
> +struct drm_i915_gem_vm_control {
> + __u64 extensions;
> + __u32 flags;
> + __u32 id;
> +};
> +
> struct drm_i915_reg_read {
> /*
> * Register offset.
> @@ -1540,7 +1564,19 @@ struct drm_i915_gem_context_param {
> * On creation, all new contexts are marked as recoverable.
> */
> #define I915_CONTEXT_PARAM_RECOVERABLE 0x8
> +
> + /*
> + * The id of the associated virtual memory address space (ppGTT) of
> + * this context. Can be retrieved and passed to another context
> + * (on the same fd) for both to use the same ppGTT and so share
> + * address layouts, and avoid reloading the page tables on context
> + * switches between themselves.
> + *
> + * See DRM_I915_GEM_VM_CREATE and DRM_I915_GEM_VM_DESTROY.
> + */
> +#define I915_CONTEXT_PARAM_VM 0x9
> /* Must be kept compact -- no holes and well documented */
> +
> __u64 value;
> };
>
>
Looks ready to me.
Regards,
Tvrtko
More information about the Intel-gfx
mailing list