| Message ID | 20210319223856.2983244-2-jason@jlekstrand.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: uAPI clean-ups part 2 | expand |
On Fri, Mar 19, 2021 at 5:39 PM Jason Ekstrand <jason@jlekstrand.net> wrote: > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > has never been used by any real userspace. After further digging, there is a compute-runtime PR for this. I still think we should drop it and I've updated the commit message locally with the following rationale: This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API was originally added for OpenCL but the compute-runtime PR has sat open for a year without action so we can still pull it out if we want. I argue we should drop it for three reasons: 1. If the compute-runtime PR has sat open for a year, this clearly isn't that important. 2. It's a very leaky API. Ring size is an implementation detail of the current execlist scheduler and really only makes sense there. It can't apply to the older ring-buffer scheduler on pre-execlist hardware because that's shared across all contexts and it won't apply to the GuC scheduler that's in the pipeline. 3. Having userspace set a ring size in bytes is a bad solution to the problem of having too small a ring. There is no way that userspace has the information to know how to properly set the ring size so it's just going to detect the feature and always set it to the maximum of 512K. This is what the compute-runtime PR does. The scheduler in i915, on the other hand, does have the information to make an informed choice. It could detect if the ring size is a problem and grow it itself. Or, if that's too hard, we could just increase the default size from 16K to 32K or even 64K instead of relying on userspace to do it. Let's drop this API for now and, if someone decides they really care about solving this problem, they can do it properly. > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> > --- > drivers/gpu/drm/i915/Makefile | 1 - > drivers/gpu/drm/i915/gem/i915_gem_context.c | 112 ++---------------- > drivers/gpu/drm/i915/gt/intel_context_param.c | 63 ---------- > drivers/gpu/drm/i915/gt/intel_context_param.h | 14 --- > include/uapi/drm/i915_drm.h | 20 +--- > 5 files changed, 12 insertions(+), 198 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gt/intel_context_param.c > delete mode 100644 drivers/gpu/drm/i915/gt/intel_context_param.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 921db06232c35..6825988f0328f 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -87,7 +87,6 @@ gt-y += \ > gt/gen8_ppgtt.o \ > gt/intel_breadcrumbs.o \ > gt/intel_context.o \ > - gt/intel_context_param.o \ > gt/intel_context_sseu.o \ > gt/intel_engine_cs.o \ > gt/intel_engine_heartbeat.o \ > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index 4d2f40cf237bd..c528db3b7dcf4 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -69,7 +69,6 @@ > > #include "gt/gen6_ppgtt.h" > #include "gt/intel_context.h" > -#include "gt/intel_context_param.h" > #include "gt/intel_engine_heartbeat.h" > #include "gt/intel_engine_user.h" > #include "gt/intel_execlists_submission.h" /* virtual_engine */ > @@ -744,32 +743,25 @@ __context_engines_await(const struct i915_gem_context *ctx, > return engines; > } > > -static int > +static void > context_apply_all(struct i915_gem_context *ctx, > - int (*fn)(struct intel_context *ce, void *data), > + void (*fn)(struct intel_context *ce, void *data), > void *data) > { > struct i915_gem_engines_iter it; > struct i915_gem_engines *e; > struct intel_context *ce; > - int err = 0; > > e = __context_engines_await(ctx, NULL); > - for_each_gem_engine(ce, e, it) { > - err = fn(ce, data); > - if (err) > - break; > - } > + for_each_gem_engine(ce, e, it) > + fn(ce, data); > i915_sw_fence_complete(&e->fence); > - > - return err; > } > > -static int __apply_ppgtt(struct intel_context *ce, void *vm) > +static void __apply_ppgtt(struct intel_context *ce, void *vm) > { > i915_vm_put(ce->vm); > ce->vm = i915_vm_get(vm); > - return 0; > } > > static struct i915_address_space * > @@ -809,10 +801,9 @@ static void __set_timeline(struct intel_timeline **dst, > intel_timeline_put(old); > } > > -static int __apply_timeline(struct intel_context *ce, void *timeline) > +static void __apply_timeline(struct intel_context *ce, void *timeline) > { > __set_timeline(&ce->timeline, timeline); > - return 0; > } > > static void __assign_timeline(struct i915_gem_context *ctx, > @@ -1328,63 +1319,6 @@ static int set_ppgtt(struct drm_i915_file_private *file_priv, > return err; > } > > -static int __apply_ringsize(struct intel_context *ce, void *sz) > -{ > - return intel_context_set_ring_size(ce, (unsigned long)sz); > -} > - > -static int set_ringsize(struct i915_gem_context *ctx, > - struct drm_i915_gem_context_param *args) > -{ > - if (!HAS_LOGICAL_RING_CONTEXTS(ctx->i915)) > - return -ENODEV; > - > - if (args->size) > - return -EINVAL; > - > - if (!IS_ALIGNED(args->value, I915_GTT_PAGE_SIZE)) > - return -EINVAL; > - > - if (args->value < I915_GTT_PAGE_SIZE) > - return -EINVAL; > - > - if (args->value > 128 * I915_GTT_PAGE_SIZE) > - return -EINVAL; > - > - return context_apply_all(ctx, > - __apply_ringsize, > - __intel_context_ring_size(args->value)); > -} > - > -static int __get_ringsize(struct intel_context *ce, void *arg) > -{ > - long sz; > - > - sz = intel_context_get_ring_size(ce); > - GEM_BUG_ON(sz > INT_MAX); > - > - return sz; /* stop on first engine */ > -} > - > -static int get_ringsize(struct i915_gem_context *ctx, > - struct drm_i915_gem_context_param *args) > -{ > - int sz; > - > - if (!HAS_LOGICAL_RING_CONTEXTS(ctx->i915)) > - return -ENODEV; > - > - if (args->size) > - return -EINVAL; > - > - sz = context_apply_all(ctx, __get_ringsize, NULL); > - if (sz < 0) > - return sz; > - > - args->value = sz; > - return 0; > -} > - > int > i915_gem_user_to_context_sseu(struct intel_gt *gt, > const struct drm_i915_gem_context_param_sseu *user, > @@ -1925,19 +1859,17 @@ set_persistence(struct i915_gem_context *ctx, > return __context_set_persistence(ctx, args->value); > } > > -static int __apply_priority(struct intel_context *ce, void *arg) > +static void __apply_priority(struct intel_context *ce, void *arg) > { > struct i915_gem_context *ctx = arg; > > if (!intel_engine_has_timeslices(ce->engine)) > - return 0; > + return; > > if (ctx->sched.priority >= I915_PRIORITY_NORMAL) > intel_context_set_use_semaphores(ce); > else > intel_context_clear_use_semaphores(ce); > - > - return 0; > } > > static int set_priority(struct i915_gem_context *ctx, > @@ -2030,11 +1962,8 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, > ret = set_persistence(ctx, args); > break; > > - case I915_CONTEXT_PARAM_RINGSIZE: > - ret = set_ringsize(ctx, args); > - break; > - > case I915_CONTEXT_PARAM_BAN_PERIOD: > + case I915_CONTEXT_PARAM_RINGSIZE: > default: > ret = -EINVAL; > break; > @@ -2062,18 +1991,6 @@ 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) > -{ > - 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) > { > @@ -2118,12 +2035,6 @@ static int clone_engines(struct i915_gem_context *dst, > } > > 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); > @@ -2483,11 +2394,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, > args->value = i915_gem_context_is_persistent(ctx); > break; > > - case I915_CONTEXT_PARAM_RINGSIZE: > - ret = get_ringsize(ctx, args); > - break; > - > case I915_CONTEXT_PARAM_BAN_PERIOD: > + case I915_CONTEXT_PARAM_RINGSIZE: > default: > ret = -EINVAL; > break; > diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.c b/drivers/gpu/drm/i915/gt/intel_context_param.c > deleted file mode 100644 > index 65dcd090245d6..0000000000000 > --- a/drivers/gpu/drm/i915/gt/intel_context_param.c > +++ /dev/null > @@ -1,63 +0,0 @@ > -// SPDX-License-Identifier: MIT > -/* > - * Copyright © 2019 Intel Corporation > - */ > - > -#include "i915_active.h" > -#include "intel_context.h" > -#include "intel_context_param.h" > -#include "intel_ring.h" > - > -int intel_context_set_ring_size(struct intel_context *ce, long sz) > -{ > - int err; > - > - if (intel_context_lock_pinned(ce)) > - return -EINTR; > - > - err = i915_active_wait(&ce->active); > - if (err < 0) > - goto unlock; > - > - if (intel_context_is_pinned(ce)) { > - err = -EBUSY; /* In active use, come back later! */ > - goto unlock; > - } > - > - if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) { > - struct intel_ring *ring; > - > - /* Replace the existing ringbuffer */ > - ring = intel_engine_create_ring(ce->engine, sz); > - if (IS_ERR(ring)) { > - err = PTR_ERR(ring); > - goto unlock; > - } > - > - intel_ring_put(ce->ring); > - ce->ring = ring; > - > - /* Context image will be updated on next pin */ > - } else { > - ce->ring = __intel_context_ring_size(sz); > - } > - > -unlock: > - intel_context_unlock_pinned(ce); > - return err; > -} > - > -long intel_context_get_ring_size(struct intel_context *ce) > -{ > - long sz = (unsigned long)READ_ONCE(ce->ring); > - > - if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) { > - if (intel_context_lock_pinned(ce)) > - return -EINTR; > - > - sz = ce->ring->size; > - intel_context_unlock_pinned(ce); > - } > - > - return sz; > -} > diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.h b/drivers/gpu/drm/i915/gt/intel_context_param.h > deleted file mode 100644 > index f053d8633fe2a..0000000000000 > --- a/drivers/gpu/drm/i915/gt/intel_context_param.h > +++ /dev/null > @@ -1,14 +0,0 @@ > -/* SPDX-License-Identifier: MIT */ > -/* > - * Copyright © 2019 Intel Corporation > - */ > - > -#ifndef INTEL_CONTEXT_PARAM_H > -#define INTEL_CONTEXT_PARAM_H > - > -struct intel_context; > - > -int intel_context_set_ring_size(struct intel_context *ce, long sz); > -long intel_context_get_ring_size(struct intel_context *ce); > - > -#endif /* INTEL_CONTEXT_PARAM_H */ > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index ddc47bbf48b6d..a80eb2bcd22bc 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -1675,24 +1675,8 @@ struct drm_i915_gem_context_param { > */ > #define I915_CONTEXT_PARAM_PERSISTENCE 0xb > > -/* > - * I915_CONTEXT_PARAM_RINGSIZE: > - * > - * Sets the size of the CS ringbuffer to use for logical ring contexts. This > - * applies a limit of how many batches can be queued to HW before the caller > - * is blocked due to lack of space for more commands. > - * > - * Only reliably possible to be set prior to first use, i.e. during > - * construction. At any later point, the current execution must be flushed as > - * the ring can only be changed while the context is idle. Note, the ringsize > - * can be specified as a constructor property, see > - * I915_CONTEXT_CREATE_EXT_SETPARAM, but can also be set later if required. > - * > - * Only applies to the current set of engine and lost when those engines > - * are replaced by a new mapping (see I915_CONTEXT_PARAM_ENGINES). > - * > - * Must be between 4 - 512 KiB, in intervals of page size [4 KiB]. > - * Default is 16 KiB. > +/* This API has been removed. On the off chance someone somewhere has > + * attempted to use it, never re-use this context param number. > */ > #define I915_CONTEXT_PARAM_RINGSIZE 0xc > /* Must be kept compact -- no holes and well documented */ > -- > 2.29.2 >
On Sat, 20 Mar 2021 at 14:48, Jason Ekstrand <jason@jlekstrand.net> wrote: > > On Fri, Mar 19, 2021 at 5:39 PM Jason Ekstrand <jason@jlekstrand.net> wrote: > > > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > > has never been used by any real userspace. > > After further digging, there is a compute-runtime PR for this. I > still think we should drop it and I've updated the commit message > locally with the following rationale: > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > was originally added for OpenCL but the compute-runtime PR has sat open > for a year without action so we can still pull it out if we want. I > argue we should drop it for three reasons: > > 1. If the compute-runtime PR has sat open for a year, this clearly > isn't that important. > > 2. It's a very leaky API. Ring size is an implementation detail of the > current execlist scheduler and really only makes sense there. It > can't apply to the older ring-buffer scheduler on pre-execlist > hardware because that's shared across all contexts and it won't > apply to the GuC scheduler that's in the pipeline. Just a drive-by-comment. I thought the lrc model was shared between execlists and the GuC, so in both cases we get something like a ring per engine per context which the driver can emit commands into. Why doesn't ring size still apply with the GuC scheduler?
On Sat, 20 Mar 2021, Jason Ekstrand <jason@jlekstrand.net> wrote:
> This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API
Small nit, I think it would be useful to reference commits with the
citation style:
88be76cdafc7 ("drm/i915: Allow userspace to specify ringsize on construction")
I use this monster in my .gitconfig:
[alias]
cite = "!f() { git log -1 '--pretty=format:%H (\"%s\")%n' $1 | sed -e 's/\\([0-f]\\{12\\}\\)[0-f]*/\\1/'; }; f"
With that, 'git cite <committish>' will give you the nice reference with
12 chars of sha1 regardless of core.abbrev config.
BR,
Jani.
On Mon, Mar 22, 2021 at 5:52 AM Matthew Auld <matthew.william.auld@gmail.com> wrote: > > On Sat, 20 Mar 2021 at 14:48, Jason Ekstrand <jason@jlekstrand.net> wrote: > > > > On Fri, Mar 19, 2021 at 5:39 PM Jason Ekstrand <jason@jlekstrand.net> wrote: > > > > > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > > > has never been used by any real userspace. > > > > After further digging, there is a compute-runtime PR for this. I > > still think we should drop it and I've updated the commit message > > locally with the following rationale: > > > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > > was originally added for OpenCL but the compute-runtime PR has sat open > > for a year without action so we can still pull it out if we want. I > > argue we should drop it for three reasons: > > > > 1. If the compute-runtime PR has sat open for a year, this clearly > > isn't that important. > > > > 2. It's a very leaky API. Ring size is an implementation detail of the > > current execlist scheduler and really only makes sense there. It > > can't apply to the older ring-buffer scheduler on pre-execlist > > hardware because that's shared across all contexts and it won't > > apply to the GuC scheduler that's in the pipeline. > > Just a drive-by-comment. I thought the lrc model was shared between > execlists and the GuC, so in both cases we get something like a ring > per engine per context which the driver can emit commands into. Why > doesn't ring size still apply with the GuC scheduler? Even if they both have a ring in some sense, the number of bytes which correspond to a single request is going to be different and that difference isn't visible to userspace so bytes is the wrong unit. --Jason
On Mon, Mar 22, 2021 at 7:01 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Sat, 20 Mar 2021, Jason Ekstrand <jason@jlekstrand.net> wrote: > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > > Small nit, I think it would be useful to reference commits with the > citation style: > > 88be76cdafc7 ("drm/i915: Allow userspace to specify ringsize on construction") Done. > > I use this monster in my .gitconfig: > > [alias] > cite = "!f() { git log -1 '--pretty=format:%H (\"%s\")%n' $1 | sed -e 's/\\([0-f]\\{12\\}\\)[0-f]*/\\1/'; }; f" Thanks for the tip! > With that, 'git cite <committish>' will give you the nice reference with > 12 chars of sha1 regardless of core.abbrev config. > > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Graphics Center
On Mon, Mar 22, 2021 at 5:01 PM Jason Ekstrand <jason@jlekstrand.net> wrote: > > On Mon, Mar 22, 2021 at 7:01 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > > On Sat, 20 Mar 2021, Jason Ekstrand <jason@jlekstrand.net> wrote: > > > This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API > > > > Small nit, I think it would be useful to reference commits with the > > citation style: > > > > 88be76cdafc7 ("drm/i915: Allow userspace to specify ringsize on construction") > > Done. > > > > > I use this monster in my .gitconfig: > > > > [alias] > > cite = "!f() { git log -1 '--pretty=format:%H (\"%s\")%n' $1 | sed -e 's/\\([0-f]\\{12\\}\\)[0-f]*/\\1/'; }; f" > > Thanks for the tip! dim script from maintainer-tools has a bunch of these helpers (andother one is dim fixes for generating Cc: lists) which should work even without commit rights and all that set up: https://gitlab.freedesktop.org/drm/maintainer-tools/-/blob/master/dim Cheers, Daniel > > > With that, 'git cite <committish>' will give you the nice reference with > > 12 chars of sha1 regardless of core.abbrev config. > > > > > > BR, > > Jani. > > > > -- > > Jani Nikula, Intel Open Source Graphics Center > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 921db06232c35..6825988f0328f 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -87,7 +87,6 @@ gt-y += \ gt/gen8_ppgtt.o \ gt/intel_breadcrumbs.o \ gt/intel_context.o \ - gt/intel_context_param.o \ gt/intel_context_sseu.o \ gt/intel_engine_cs.o \ gt/intel_engine_heartbeat.o \ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 4d2f40cf237bd..c528db3b7dcf4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -69,7 +69,6 @@ #include "gt/gen6_ppgtt.h" #include "gt/intel_context.h" -#include "gt/intel_context_param.h" #include "gt/intel_engine_heartbeat.h" #include "gt/intel_engine_user.h" #include "gt/intel_execlists_submission.h" /* virtual_engine */ @@ -744,32 +743,25 @@ __context_engines_await(const struct i915_gem_context *ctx, return engines; } -static int +static void context_apply_all(struct i915_gem_context *ctx, - int (*fn)(struct intel_context *ce, void *data), + void (*fn)(struct intel_context *ce, void *data), void *data) { struct i915_gem_engines_iter it; struct i915_gem_engines *e; struct intel_context *ce; - int err = 0; e = __context_engines_await(ctx, NULL); - for_each_gem_engine(ce, e, it) { - err = fn(ce, data); - if (err) - break; - } + for_each_gem_engine(ce, e, it) + fn(ce, data); i915_sw_fence_complete(&e->fence); - - return err; } -static int __apply_ppgtt(struct intel_context *ce, void *vm) +static void __apply_ppgtt(struct intel_context *ce, void *vm) { i915_vm_put(ce->vm); ce->vm = i915_vm_get(vm); - return 0; } static struct i915_address_space * @@ -809,10 +801,9 @@ static void __set_timeline(struct intel_timeline **dst, intel_timeline_put(old); } -static int __apply_timeline(struct intel_context *ce, void *timeline) +static void __apply_timeline(struct intel_context *ce, void *timeline) { __set_timeline(&ce->timeline, timeline); - return 0; } static void __assign_timeline(struct i915_gem_context *ctx, @@ -1328,63 +1319,6 @@ static int set_ppgtt(struct drm_i915_file_private *file_priv, return err; } -static int __apply_ringsize(struct intel_context *ce, void *sz) -{ - return intel_context_set_ring_size(ce, (unsigned long)sz); -} - -static int set_ringsize(struct i915_gem_context *ctx, - struct drm_i915_gem_context_param *args) -{ - if (!HAS_LOGICAL_RING_CONTEXTS(ctx->i915)) - return -ENODEV; - - if (args->size) - return -EINVAL; - - if (!IS_ALIGNED(args->value, I915_GTT_PAGE_SIZE)) - return -EINVAL; - - if (args->value < I915_GTT_PAGE_SIZE) - return -EINVAL; - - if (args->value > 128 * I915_GTT_PAGE_SIZE) - return -EINVAL; - - return context_apply_all(ctx, - __apply_ringsize, - __intel_context_ring_size(args->value)); -} - -static int __get_ringsize(struct intel_context *ce, void *arg) -{ - long sz; - - sz = intel_context_get_ring_size(ce); - GEM_BUG_ON(sz > INT_MAX); - - return sz; /* stop on first engine */ -} - -static int get_ringsize(struct i915_gem_context *ctx, - struct drm_i915_gem_context_param *args) -{ - int sz; - - if (!HAS_LOGICAL_RING_CONTEXTS(ctx->i915)) - return -ENODEV; - - if (args->size) - return -EINVAL; - - sz = context_apply_all(ctx, __get_ringsize, NULL); - if (sz < 0) - return sz; - - args->value = sz; - return 0; -} - int i915_gem_user_to_context_sseu(struct intel_gt *gt, const struct drm_i915_gem_context_param_sseu *user, @@ -1925,19 +1859,17 @@ set_persistence(struct i915_gem_context *ctx, return __context_set_persistence(ctx, args->value); } -static int __apply_priority(struct intel_context *ce, void *arg) +static void __apply_priority(struct intel_context *ce, void *arg) { struct i915_gem_context *ctx = arg; if (!intel_engine_has_timeslices(ce->engine)) - return 0; + return; if (ctx->sched.priority >= I915_PRIORITY_NORMAL) intel_context_set_use_semaphores(ce); else intel_context_clear_use_semaphores(ce); - - return 0; } static int set_priority(struct i915_gem_context *ctx, @@ -2030,11 +1962,8 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, ret = set_persistence(ctx, args); break; - case I915_CONTEXT_PARAM_RINGSIZE: - ret = set_ringsize(ctx, args); - break; - case I915_CONTEXT_PARAM_BAN_PERIOD: + case I915_CONTEXT_PARAM_RINGSIZE: default: ret = -EINVAL; break; @@ -2062,18 +1991,6 @@ 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) -{ - 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) { @@ -2118,12 +2035,6 @@ static int clone_engines(struct i915_gem_context *dst, } 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); @@ -2483,11 +2394,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, args->value = i915_gem_context_is_persistent(ctx); break; - case I915_CONTEXT_PARAM_RINGSIZE: - ret = get_ringsize(ctx, args); - break; - case I915_CONTEXT_PARAM_BAN_PERIOD: + case I915_CONTEXT_PARAM_RINGSIZE: default: ret = -EINVAL; break; diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.c b/drivers/gpu/drm/i915/gt/intel_context_param.c deleted file mode 100644 index 65dcd090245d6..0000000000000 --- a/drivers/gpu/drm/i915/gt/intel_context_param.c +++ /dev/null @@ -1,63 +0,0 @@ -// SPDX-License-Identifier: MIT -/* - * Copyright © 2019 Intel Corporation - */ - -#include "i915_active.h" -#include "intel_context.h" -#include "intel_context_param.h" -#include "intel_ring.h" - -int intel_context_set_ring_size(struct intel_context *ce, long sz) -{ - int err; - - if (intel_context_lock_pinned(ce)) - return -EINTR; - - err = i915_active_wait(&ce->active); - if (err < 0) - goto unlock; - - if (intel_context_is_pinned(ce)) { - err = -EBUSY; /* In active use, come back later! */ - goto unlock; - } - - if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) { - struct intel_ring *ring; - - /* Replace the existing ringbuffer */ - ring = intel_engine_create_ring(ce->engine, sz); - if (IS_ERR(ring)) { - err = PTR_ERR(ring); - goto unlock; - } - - intel_ring_put(ce->ring); - ce->ring = ring; - - /* Context image will be updated on next pin */ - } else { - ce->ring = __intel_context_ring_size(sz); - } - -unlock: - intel_context_unlock_pinned(ce); - return err; -} - -long intel_context_get_ring_size(struct intel_context *ce) -{ - long sz = (unsigned long)READ_ONCE(ce->ring); - - if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) { - if (intel_context_lock_pinned(ce)) - return -EINTR; - - sz = ce->ring->size; - intel_context_unlock_pinned(ce); - } - - return sz; -} diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.h b/drivers/gpu/drm/i915/gt/intel_context_param.h deleted file mode 100644 index f053d8633fe2a..0000000000000 --- a/drivers/gpu/drm/i915/gt/intel_context_param.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: MIT */ -/* - * Copyright © 2019 Intel Corporation - */ - -#ifndef INTEL_CONTEXT_PARAM_H -#define INTEL_CONTEXT_PARAM_H - -struct intel_context; - -int intel_context_set_ring_size(struct intel_context *ce, long sz); -long intel_context_get_ring_size(struct intel_context *ce); - -#endif /* INTEL_CONTEXT_PARAM_H */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ddc47bbf48b6d..a80eb2bcd22bc 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1675,24 +1675,8 @@ struct drm_i915_gem_context_param { */ #define I915_CONTEXT_PARAM_PERSISTENCE 0xb -/* - * I915_CONTEXT_PARAM_RINGSIZE: - * - * Sets the size of the CS ringbuffer to use for logical ring contexts. This - * applies a limit of how many batches can be queued to HW before the caller - * is blocked due to lack of space for more commands. - * - * Only reliably possible to be set prior to first use, i.e. during - * construction. At any later point, the current execution must be flushed as - * the ring can only be changed while the context is idle. Note, the ringsize - * can be specified as a constructor property, see - * I915_CONTEXT_CREATE_EXT_SETPARAM, but can also be set later if required. - * - * Only applies to the current set of engine and lost when those engines - * are replaced by a new mapping (see I915_CONTEXT_PARAM_ENGINES). - * - * Must be between 4 - 512 KiB, in intervals of page size [4 KiB]. - * Default is 16 KiB. +/* This API has been removed. On the off chance someone somewhere has + * attempted to use it, never re-use this context param number. */ #define I915_CONTEXT_PARAM_RINGSIZE 0xc /* Must be kept compact -- no holes and well documented */
This reverts commit 88be76cdafc7e60e2e4ed883bfe7e8dd7f35fa3a. This API has never been used by any real userspace. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> --- drivers/gpu/drm/i915/Makefile | 1 - drivers/gpu/drm/i915/gem/i915_gem_context.c | 112 ++---------------- drivers/gpu/drm/i915/gt/intel_context_param.c | 63 ---------- drivers/gpu/drm/i915/gt/intel_context_param.h | 14 --- include/uapi/drm/i915_drm.h | 20 +--- 5 files changed, 12 insertions(+), 198 deletions(-) delete mode 100644 drivers/gpu/drm/i915/gt/intel_context_param.c delete mode 100644 drivers/gpu/drm/i915/gt/intel_context_param.h