| Message ID | 20210317234014.2271006-5-jason@jlekstrand.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Clean up some of the i915 uAPI (v6) | expand |
On 17/03/2021 23:40, Jason Ekstrand wrote: > It's never been used by any real userspace. It's used by IGT as a > short-cut for sharing VMs and other stuff between contexts. I haven't checked if userspace uses this so assuming that is fine, the only thing that remains is preparing the IGTs for the transition which cannot be avoided. Regards, Tvrtko > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_context.c | 217 +------------------- > include/uapi/drm/i915_drm.h | 16 +- > 2 files changed, 6 insertions(+), 227 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index ca37d93ef5e78..92256f4e50764 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -2103,225 +2103,14 @@ static int create_setparam(struct i915_user_extension __user *ext, void *data) > return ctx_setparam(arg->fpriv, arg->ctx, &local.param); > } > > -static int copy_ring_size(struct intel_context *dst, > - struct intel_context *src) > +static int invalid_ext(struct i915_user_extension __user *ext, void *data) > { > - long sz; > - > - sz = intel_context_get_ring_size(src); > - if (sz < 0) > - return sz; > - > - return intel_context_set_ring_size(dst, sz); > -} > - > -static int clone_engines(struct i915_gem_context *dst, > - struct i915_gem_context *src) > -{ > - struct i915_gem_engines *clone, *e; > - bool user_engines; > - unsigned long n; > - > - e = __context_engines_await(src, &user_engines); > - if (!e) > - return -ENOENT; > - > - clone = alloc_engines(e->num_engines); > - if (!clone) > - goto err_unlock; > - > - for (n = 0; n < e->num_engines; n++) { > - struct intel_engine_cs *engine; > - > - if (!e->engines[n]) { > - clone->engines[n] = NULL; > - continue; > - } > - engine = e->engines[n]->engine; > - > - /* > - * Virtual engines are singletons; they can only exist > - * inside a single context, because they embed their > - * HW context... As each virtual context implies a single > - * timeline (each engine can only dequeue a single request > - * at any time), it would be surprising for two contexts > - * to use the same engine. So let's create a copy of > - * the virtual engine instead. > - */ > - if (intel_engine_is_virtual(engine)) > - clone->engines[n] = > - intel_execlists_clone_virtual(engine); > - else > - clone->engines[n] = intel_context_create(engine); > - if (IS_ERR_OR_NULL(clone->engines[n])) { > - __free_engines(clone, n); > - goto err_unlock; > - } > - > - intel_context_set_gem(clone->engines[n], dst); > - > - /* Copy across the preferred ringsize */ > - if (copy_ring_size(clone->engines[n], e->engines[n])) { > - __free_engines(clone, n + 1); > - goto err_unlock; > - } > - } > - clone->num_engines = n; > - i915_sw_fence_complete(&e->fence); > - > - /* Serialised by constructor */ > - engines_idle_release(dst, rcu_replace_pointer(dst->engines, clone, 1)); > - if (user_engines) > - i915_gem_context_set_user_engines(dst); > - else > - i915_gem_context_clear_user_engines(dst); > - return 0; > - > -err_unlock: > - i915_sw_fence_complete(&e->fence); > - return -ENOMEM; > -} > - > -static int clone_flags(struct i915_gem_context *dst, > - struct i915_gem_context *src) > -{ > - dst->user_flags = src->user_flags; > - return 0; > -} > - > -static int clone_schedattr(struct i915_gem_context *dst, > - struct i915_gem_context *src) > -{ > - dst->sched = src->sched; > - return 0; > -} > - > -static int clone_sseu(struct i915_gem_context *dst, > - struct i915_gem_context *src) > -{ > - struct i915_gem_engines *e = i915_gem_context_lock_engines(src); > - struct i915_gem_engines *clone; > - unsigned long n; > - int err; > - > - /* no locking required; sole access under constructor*/ > - clone = __context_engines_static(dst); > - if (e->num_engines != clone->num_engines) { > - err = -EINVAL; > - goto unlock; > - } > - > - for (n = 0; n < e->num_engines; n++) { > - struct intel_context *ce = e->engines[n]; > - > - if (clone->engines[n]->engine->class != ce->engine->class) { > - /* Must have compatible engine maps! */ > - err = -EINVAL; > - goto unlock; > - } > - > - /* serialises with set_sseu */ > - err = intel_context_lock_pinned(ce); > - if (err) > - goto unlock; > - > - clone->engines[n]->sseu = ce->sseu; > - intel_context_unlock_pinned(ce); > - } > - > - err = 0; > -unlock: > - i915_gem_context_unlock_engines(src); > - return err; > -} > - > -static int clone_timeline(struct i915_gem_context *dst, > - struct i915_gem_context *src) > -{ > - if (src->timeline) > - __assign_timeline(dst, src->timeline); > - > - return 0; > -} > - > -static int clone_vm(struct i915_gem_context *dst, > - struct i915_gem_context *src) > -{ > - struct i915_address_space *vm; > - int err = 0; > - > - if (!rcu_access_pointer(src->vm)) > - return 0; > - > - rcu_read_lock(); > - vm = context_get_vm_rcu(src); > - rcu_read_unlock(); > - > - if (!mutex_lock_interruptible(&dst->mutex)) { > - __assign_ppgtt(dst, vm); > - mutex_unlock(&dst->mutex); > - } else { > - err = -EINTR; > - } > - > - i915_vm_put(vm); > - return err; > -} > - > -static int create_clone(struct i915_user_extension __user *ext, void *data) > -{ > - static int (* const fn[])(struct i915_gem_context *dst, > - struct i915_gem_context *src) = { > -#define MAP(x, y) [ilog2(I915_CONTEXT_CLONE_##x)] = y > - MAP(ENGINES, clone_engines), > - MAP(FLAGS, clone_flags), > - MAP(SCHEDATTR, clone_schedattr), > - MAP(SSEU, clone_sseu), > - MAP(TIMELINE, clone_timeline), > - MAP(VM, clone_vm), > -#undef MAP > - }; > - struct drm_i915_gem_context_create_ext_clone local; > - const struct create_ext *arg = data; > - struct i915_gem_context *dst = arg->ctx; > - struct i915_gem_context *src; > - int err, bit; > - > - if (copy_from_user(&local, ext, sizeof(local))) > - return -EFAULT; > - > - BUILD_BUG_ON(GENMASK(BITS_PER_TYPE(local.flags) - 1, ARRAY_SIZE(fn)) != > - I915_CONTEXT_CLONE_UNKNOWN); > - > - if (local.flags & I915_CONTEXT_CLONE_UNKNOWN) > - return -EINVAL; > - > - if (local.rsvd) > - return -EINVAL; > - > - rcu_read_lock(); > - src = __i915_gem_context_lookup_rcu(arg->fpriv, local.clone_id); > - rcu_read_unlock(); > - if (!src) > - return -ENOENT; > - > - GEM_BUG_ON(src == dst); > - > - for (bit = 0; bit < ARRAY_SIZE(fn); bit++) { > - if (!(local.flags & BIT(bit))) > - continue; > - > - err = fn[bit](dst, src); > - if (err) > - return err; > - } > - > - return 0; > + return -EINVAL; > } > > static const i915_user_extension_fn create_extensions[] = { > [I915_CONTEXT_CREATE_EXT_SETPARAM] = create_setparam, > - [I915_CONTEXT_CREATE_EXT_CLONE] = create_clone, > + [I915_CONTEXT_CREATE_EXT_CLONE] = invalid_ext, > }; > > static bool client_is_banned(struct drm_i915_file_private *file_priv) > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index ddc47bbf48b6d..0ed1b88afedd5 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -1853,20 +1853,10 @@ struct drm_i915_gem_context_create_ext_setparam { > struct drm_i915_gem_context_param param; > }; > > -struct drm_i915_gem_context_create_ext_clone { > +/* This API has been removed. On the off chance someone somewhere has > + * attempted to use it, never re-use this extension number. > + */ > #define I915_CONTEXT_CREATE_EXT_CLONE 1 > - struct i915_user_extension base; > - __u32 clone_id; > - __u32 flags; > -#define I915_CONTEXT_CLONE_ENGINES (1u << 0) > -#define I915_CONTEXT_CLONE_FLAGS (1u << 1) > -#define I915_CONTEXT_CLONE_SCHEDATTR (1u << 2) > -#define I915_CONTEXT_CLONE_SSEU (1u << 3) > -#define I915_CONTEXT_CLONE_TIMELINE (1u << 4) > -#define I915_CONTEXT_CLONE_VM (1u << 5) > -#define I915_CONTEXT_CLONE_UNKNOWN -(I915_CONTEXT_CLONE_VM << 1) > - __u64 rsvd; > -}; > > struct drm_i915_gem_context_destroy { > __u32 ctx_id; >
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index ca37d93ef5e78..92256f4e50764 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -2103,225 +2103,14 @@ static int create_setparam(struct i915_user_extension __user *ext, void *data) return ctx_setparam(arg->fpriv, arg->ctx, &local.param); } -static int copy_ring_size(struct intel_context *dst, - struct intel_context *src) +static int invalid_ext(struct i915_user_extension __user *ext, void *data) { - long sz; - - sz = intel_context_get_ring_size(src); - if (sz < 0) - return sz; - - return intel_context_set_ring_size(dst, sz); -} - -static int clone_engines(struct i915_gem_context *dst, - struct i915_gem_context *src) -{ - struct i915_gem_engines *clone, *e; - bool user_engines; - unsigned long n; - - e = __context_engines_await(src, &user_engines); - if (!e) - return -ENOENT; - - clone = alloc_engines(e->num_engines); - if (!clone) - goto err_unlock; - - for (n = 0; n < e->num_engines; n++) { - struct intel_engine_cs *engine; - - if (!e->engines[n]) { - clone->engines[n] = NULL; - continue; - } - engine = e->engines[n]->engine; - - /* - * Virtual engines are singletons; they can only exist - * inside a single context, because they embed their - * HW context... As each virtual context implies a single - * timeline (each engine can only dequeue a single request - * at any time), it would be surprising for two contexts - * to use the same engine. So let's create a copy of - * the virtual engine instead. - */ - if (intel_engine_is_virtual(engine)) - clone->engines[n] = - intel_execlists_clone_virtual(engine); - else - clone->engines[n] = intel_context_create(engine); - if (IS_ERR_OR_NULL(clone->engines[n])) { - __free_engines(clone, n); - goto err_unlock; - } - - intel_context_set_gem(clone->engines[n], dst); - - /* Copy across the preferred ringsize */ - if (copy_ring_size(clone->engines[n], e->engines[n])) { - __free_engines(clone, n + 1); - goto err_unlock; - } - } - clone->num_engines = n; - i915_sw_fence_complete(&e->fence); - - /* Serialised by constructor */ - engines_idle_release(dst, rcu_replace_pointer(dst->engines, clone, 1)); - if (user_engines) - i915_gem_context_set_user_engines(dst); - else - i915_gem_context_clear_user_engines(dst); - return 0; - -err_unlock: - i915_sw_fence_complete(&e->fence); - return -ENOMEM; -} - -static int clone_flags(struct i915_gem_context *dst, - struct i915_gem_context *src) -{ - dst->user_flags = src->user_flags; - return 0; -} - -static int clone_schedattr(struct i915_gem_context *dst, - struct i915_gem_context *src) -{ - dst->sched = src->sched; - return 0; -} - -static int clone_sseu(struct i915_gem_context *dst, - struct i915_gem_context *src) -{ - struct i915_gem_engines *e = i915_gem_context_lock_engines(src); - struct i915_gem_engines *clone; - unsigned long n; - int err; - - /* no locking required; sole access under constructor*/ - clone = __context_engines_static(dst); - if (e->num_engines != clone->num_engines) { - err = -EINVAL; - goto unlock; - } - - for (n = 0; n < e->num_engines; n++) { - struct intel_context *ce = e->engines[n]; - - if (clone->engines[n]->engine->class != ce->engine->class) { - /* Must have compatible engine maps! */ - err = -EINVAL; - goto unlock; - } - - /* serialises with set_sseu */ - err = intel_context_lock_pinned(ce); - if (err) - goto unlock; - - clone->engines[n]->sseu = ce->sseu; - intel_context_unlock_pinned(ce); - } - - err = 0; -unlock: - i915_gem_context_unlock_engines(src); - return err; -} - -static int clone_timeline(struct i915_gem_context *dst, - struct i915_gem_context *src) -{ - if (src->timeline) - __assign_timeline(dst, src->timeline); - - return 0; -} - -static int clone_vm(struct i915_gem_context *dst, - struct i915_gem_context *src) -{ - struct i915_address_space *vm; - int err = 0; - - if (!rcu_access_pointer(src->vm)) - return 0; - - rcu_read_lock(); - vm = context_get_vm_rcu(src); - rcu_read_unlock(); - - if (!mutex_lock_interruptible(&dst->mutex)) { - __assign_ppgtt(dst, vm); - mutex_unlock(&dst->mutex); - } else { - err = -EINTR; - } - - i915_vm_put(vm); - return err; -} - -static int create_clone(struct i915_user_extension __user *ext, void *data) -{ - static int (* const fn[])(struct i915_gem_context *dst, - struct i915_gem_context *src) = { -#define MAP(x, y) [ilog2(I915_CONTEXT_CLONE_##x)] = y - MAP(ENGINES, clone_engines), - MAP(FLAGS, clone_flags), - MAP(SCHEDATTR, clone_schedattr), - MAP(SSEU, clone_sseu), - MAP(TIMELINE, clone_timeline), - MAP(VM, clone_vm), -#undef MAP - }; - struct drm_i915_gem_context_create_ext_clone local; - const struct create_ext *arg = data; - struct i915_gem_context *dst = arg->ctx; - struct i915_gem_context *src; - int err, bit; - - if (copy_from_user(&local, ext, sizeof(local))) - return -EFAULT; - - BUILD_BUG_ON(GENMASK(BITS_PER_TYPE(local.flags) - 1, ARRAY_SIZE(fn)) != - I915_CONTEXT_CLONE_UNKNOWN); - - if (local.flags & I915_CONTEXT_CLONE_UNKNOWN) - return -EINVAL; - - if (local.rsvd) - return -EINVAL; - - rcu_read_lock(); - src = __i915_gem_context_lookup_rcu(arg->fpriv, local.clone_id); - rcu_read_unlock(); - if (!src) - return -ENOENT; - - GEM_BUG_ON(src == dst); - - for (bit = 0; bit < ARRAY_SIZE(fn); bit++) { - if (!(local.flags & BIT(bit))) - continue; - - err = fn[bit](dst, src); - if (err) - return err; - } - - return 0; + return -EINVAL; } static const i915_user_extension_fn create_extensions[] = { [I915_CONTEXT_CREATE_EXT_SETPARAM] = create_setparam, - [I915_CONTEXT_CREATE_EXT_CLONE] = create_clone, + [I915_CONTEXT_CREATE_EXT_CLONE] = invalid_ext, }; static bool client_is_banned(struct drm_i915_file_private *file_priv) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ddc47bbf48b6d..0ed1b88afedd5 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1853,20 +1853,10 @@ struct drm_i915_gem_context_create_ext_setparam { struct drm_i915_gem_context_param param; }; -struct drm_i915_gem_context_create_ext_clone { +/* This API has been removed. On the off chance someone somewhere has + * attempted to use it, never re-use this extension number. + */ #define I915_CONTEXT_CREATE_EXT_CLONE 1 - struct i915_user_extension base; - __u32 clone_id; - __u32 flags; -#define I915_CONTEXT_CLONE_ENGINES (1u << 0) -#define I915_CONTEXT_CLONE_FLAGS (1u << 1) -#define I915_CONTEXT_CLONE_SCHEDATTR (1u << 2) -#define I915_CONTEXT_CLONE_SSEU (1u << 3) -#define I915_CONTEXT_CLONE_TIMELINE (1u << 4) -#define I915_CONTEXT_CLONE_VM (1u << 5) -#define I915_CONTEXT_CLONE_UNKNOWN -(I915_CONTEXT_CLONE_VM << 1) - __u64 rsvd; -}; struct drm_i915_gem_context_destroy { __u32 ctx_id;
It's never been used by any real userspace. It's used by IGT as a short-cut for sharing VMs and other stuff between contexts. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 217 +------------------- include/uapi/drm/i915_drm.h | 16 +- 2 files changed, 6 insertions(+), 227 deletions(-)