* [PATCH] RFC drm/i915: Allow userspace to specify ringsize on construction
@ 2019-10-10 14:23 Chris Wilson
2019-10-10 16:27 ` ✗ Fi.CI.BUILD: failure for " Patchwork
2019-10-21 10:37 ` [PATCH] " Janusz Krzysztofik
0 siblings, 2 replies; 3+ messages in thread
From: Chris Wilson @ 2019-10-10 14:23 UTC (permalink / raw)
To: intel-gfx
No good reason why we must always use a static ringsize, so let
userspace select one during construction.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 83 +++++++++++++++++++--
include/uapi/drm/i915_drm.h | 12 +++
2 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 46e5b3b53288..9635e377c8ae 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -455,23 +455,30 @@ __create_context(struct drm_i915_private *i915)
return ERR_PTR(err);
}
-static void
+static int
context_apply_all(struct i915_gem_context *ctx,
- void (*fn)(struct intel_context *ce, void *data),
+ int (*fn)(struct intel_context *ce, void *data),
void *data)
{
struct i915_gem_engines_iter it;
struct intel_context *ce;
+ int err = 0;
- for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it)
- fn(ce, data);
+ for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
+ err = fn(ce, data);
+ if (err)
+ break;
+ }
i915_gem_context_unlock_engines(ctx);
+
+ return err;
}
-static void __apply_ppgtt(struct intel_context *ce, void *vm)
+static int __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 *
@@ -509,9 +516,10 @@ static void __set_timeline(struct intel_timeline **dst,
intel_timeline_put(old);
}
-static void __apply_timeline(struct intel_context *ce, void *timeline)
+static int __apply_timeline(struct intel_context *ce, void *timeline)
{
__set_timeline(&ce->timeline, timeline);
+ return 0;
}
static void __assign_timeline(struct i915_gem_context *ctx,
@@ -1086,6 +1094,65 @@ static int set_ppgtt(struct drm_i915_file_private *file_priv,
return err;
}
+static int __apply_ringsize(struct intel_context *ce, void *sz)
+{
+ int err = 0;
+
+ if (intel_context_lock_pinned(ce))
+ return -EINTR;
+
+ 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,
+ (unsigned long)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 = sz;
+ }
+
+unlock:
+ intel_context_unlock_pinned(ce);
+ return err;
+}
+
+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 gen8_emit_rpcs_config(struct i915_request *rq,
struct intel_context *ce,
struct intel_sseu sseu)
@@ -1798,6 +1865,10 @@ 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:
default:
ret = -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index eb9e704d717a..e375cd2cf66b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1580,6 +1580,18 @@ struct drm_i915_gem_context_param {
* By default, new contexts allow persistence.
*/
#define I915_CONTEXT_PARAM_PERSISTENCE 0xb
+
+/*
+ *
+ * I915_CONTEXT_PARAM_RINGSIZE:
+ *
+ * Sets the size of the ringbuffer to use for logical ring contexts.
+ * Only possible to be set prior to first use, i.e. during construction.
+ * Only applies to the current set of engine and lost for those engines
+ * are replaced by a new mapping.
+ * Must be between 4 - 512 KiB.
+ */
+#define I915_CONTEXT_PARAM_RINGSIZE 0xc
/* Must be kept compact -- no holes and well documented */
__u64 value;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* ✗ Fi.CI.BUILD: failure for RFC drm/i915: Allow userspace to specify ringsize on construction
2019-10-10 14:23 [PATCH] RFC drm/i915: Allow userspace to specify ringsize on construction Chris Wilson
@ 2019-10-10 16:27 ` Patchwork
2019-10-21 10:37 ` [PATCH] " Janusz Krzysztofik
1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2019-10-10 16:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: RFC drm/i915: Allow userspace to specify ringsize on construction
URL : https://patchwork.freedesktop.org/series/67852/
State : failure
== Summary ==
Applying: RFC drm/i915: Allow userspace to specify ringsize on construction
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/gem/i915_gem_context.c
M include/uapi/drm/i915_drm.h
Falling back to patching base and 3-way merge...
Auto-merging include/uapi/drm/i915_drm.h
CONFLICT (content): Merge conflict in include/uapi/drm/i915_drm.h
Auto-merging drivers/gpu/drm/i915/gem/i915_gem_context.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gem/i915_gem_context.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 RFC drm/i915: Allow userspace to specify ringsize on construction
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RFC drm/i915: Allow userspace to specify ringsize on construction
2019-10-10 14:23 [PATCH] RFC drm/i915: Allow userspace to specify ringsize on construction Chris Wilson
2019-10-10 16:27 ` ✗ Fi.CI.BUILD: failure for " Patchwork
@ 2019-10-21 10:37 ` Janusz Krzysztofik
1 sibling, 0 replies; 3+ messages in thread
From: Janusz Krzysztofik @ 2019-10-21 10:37 UTC (permalink / raw)
To: intel-gfx
Hi Chris,
On Thursday, October 10, 2019 4:23:16 PM CEST Chris Wilson wrote:
> No good reason why we must always use a static ringsize, so let
> userspace select one during construction.
I've heard from UMD people they like this solution :-)
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 83 +++++++++++++++++++--
> include/uapi/drm/i915_drm.h | 12 +++
> 2 files changed, 89 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 46e5b3b53288..9635e377c8ae 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -455,23 +455,30 @@ __create_context(struct drm_i915_private *i915)
> return ERR_PTR(err);
> }
>
> -static void
> +static int
> context_apply_all(struct i915_gem_context *ctx,
> - void (*fn)(struct intel_context *ce, void *data),
> + int (*fn)(struct intel_context *ce, void *data),
> void *data)
> {
> struct i915_gem_engines_iter it;
> struct intel_context *ce;
> + int err = 0;
>
> - for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it)
> - fn(ce, data);
> + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
> + err = fn(ce, data);
> + if (err)
> + break;
> + }
> i915_gem_context_unlock_engines(ctx);
> +
> + return err;
> }
>
> -static void __apply_ppgtt(struct intel_context *ce, void *vm)
> +static int __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 *
> @@ -509,9 +516,10 @@ static void __set_timeline(struct intel_timeline **dst,
> intel_timeline_put(old);
> }
>
> -static void __apply_timeline(struct intel_context *ce, void *timeline)
> +static int __apply_timeline(struct intel_context *ce, void *timeline)
> {
> __set_timeline(&ce->timeline, timeline);
> + return 0;
> }
>
> static void __assign_timeline(struct i915_gem_context *ctx,
> @@ -1086,6 +1094,65 @@ static int set_ppgtt(struct drm_i915_file_private *file_priv,
> return err;
> }
>
> +static int __apply_ringsize(struct intel_context *ce, void *sz)
> +{
> + int err = 0;
> +
> + if (intel_context_lock_pinned(ce))
> + return -EINTR;
> +
> + 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,
> + (unsigned long)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 = sz;
> + }
> +
> +unlock:
> + intel_context_unlock_pinned(ce);
> + return err;
> +}
> +
> +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 gen8_emit_rpcs_config(struct i915_request *rq,
> struct intel_context *ce,
> struct intel_sseu sseu)
> @@ -1798,6 +1865,10 @@ 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:
> default:
> ret = -EINVAL;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index eb9e704d717a..e375cd2cf66b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1580,6 +1580,18 @@ struct drm_i915_gem_context_param {
> * By default, new contexts allow persistence.
> */
> #define I915_CONTEXT_PARAM_PERSISTENCE 0xb
> +
> +/*
> + *
> + * I915_CONTEXT_PARAM_RINGSIZE:
> + *
> + * Sets the size of the ringbuffer to use for logical ring contexts.
> + * Only possible to be set prior to first use, i.e. during construction.
> + * Only applies to the current set of engine and lost for those engines
> + * are replaced by a new mapping.
> + * Must be between 4 - 512 KiB.
nit: In case the requirement for the requested ring size to be a multiple of 4k may
be not obvious for users, and since we don't emit a debug message if not
satisfied, maybe we should also mention that here. Anyway, lgtm.
Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Thanks,
Janusz
> + */
> +#define I915_CONTEXT_PARAM_RINGSIZE 0xc
> /* Must be kept compact -- no holes and well documented */
>
> __u64 value;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-21 10:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 14:23 [PATCH] RFC drm/i915: Allow userspace to specify ringsize on construction Chris Wilson
2019-10-10 16:27 ` ✗ Fi.CI.BUILD: failure for " Patchwork
2019-10-21 10:37 ` [PATCH] " Janusz Krzysztofik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.