From: Jason Ekstrand <jason@jlekstrand.net> To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: Matthew Brost <matthew.brost@intel.com>, Jason Ekstrand <jason@jlekstrand.net> Subject: [PATCH 4/4] drm/i915: Implement SINGLE_TIMELINE with a syncobj Date: Fri, 19 Mar 2021 17:38:56 -0500 [thread overview] Message-ID: <20210319223856.2983244-5-jason@jlekstrand.net> (raw) In-Reply-To: <20210319223856.2983244-1-jason@jlekstrand.net> I'd love to delete the SINGLE_TIMELINE API because it leaks an implementation detail of contexts through to the API and is something that userspace can do itself, trivially. Unfortunately, it's used by the media driver so we can't do that. We can, however, do the next-best thing which is to embed a syncobj in the context and do exactly what we'd expect from userspace internally. This has a couple of advantages. One is that we're no longer leaking a detail of the current execlist scheduler which will be problematic when we try to add GuC scheduling. Second is that, together with deleting the CLONE_CONTEXT API, we should now have a 1:1 mapping between intel_context and intel_timeline which should make some of our locking mess a bit easier. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 47 ++++--------------- .../gpu/drm/i915/gem/i915_gem_context_types.h | 8 +++- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 15 ++++++ 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index f88bac19333ec..e094f4a1ca4cd 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -67,6 +67,8 @@ #include <linux/log2.h> #include <linux/nospec.h> +#include <drm/drm_syncobj.h> + #include "gt/gen6_ppgtt.h" #include "gt/intel_context.h" #include "gt/intel_engine_heartbeat.h" @@ -224,10 +226,6 @@ static void intel_context_set_gem(struct intel_context *ce, ce->vm = vm; } - GEM_BUG_ON(ce->timeline); - if (ctx->timeline) - ce->timeline = intel_timeline_get(ctx->timeline); - if (ctx->sched.priority >= I915_PRIORITY_NORMAL && intel_engine_has_timeslices(ce->engine)) __set_bit(CONTEXT_USE_SEMAPHORES, &ce->flags); @@ -344,8 +342,8 @@ void i915_gem_context_release(struct kref *ref) mutex_destroy(&ctx->engines_mutex); mutex_destroy(&ctx->lut_mutex); - if (ctx->timeline) - intel_timeline_put(ctx->timeline); + if (ctx->syncobj) + drm_syncobj_put(ctx->syncobj); put_pid(ctx->pid); mutex_destroy(&ctx->mutex); @@ -790,33 +788,11 @@ static void __assign_ppgtt(struct i915_gem_context *ctx, i915_vm_close(vm); } -static void __set_timeline(struct intel_timeline **dst, - struct intel_timeline *src) -{ - struct intel_timeline *old = *dst; - - *dst = src ? intel_timeline_get(src) : NULL; - - if (old) - intel_timeline_put(old); -} - -static void __apply_timeline(struct intel_context *ce, void *timeline) -{ - __set_timeline(&ce->timeline, timeline); -} - -static void __assign_timeline(struct i915_gem_context *ctx, - struct intel_timeline *timeline) -{ - __set_timeline(&ctx->timeline, timeline); - context_apply_all(ctx, __apply_timeline, timeline); -} - static struct i915_gem_context * i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) { struct i915_gem_context *ctx; + int ret; if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE && !HAS_EXECLISTS(i915)) @@ -845,16 +821,13 @@ i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) } if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE) { - struct intel_timeline *timeline; - - timeline = intel_timeline_create(&i915->gt); - if (IS_ERR(timeline)) { + ret = drm_syncobj_create(&ctx->syncobj, + DRM_SYNCOBJ_CREATE_SIGNALED, + NULL); + if (ret) { context_close(ctx); - return ERR_CAST(timeline); + return ERR_PTR(ret); } - - __assign_timeline(ctx, timeline); - intel_timeline_put(timeline); } trace_i915_context_create(ctx); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index 676592e27e7d2..8a5fdd163b79d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -83,7 +83,13 @@ struct i915_gem_context { struct i915_gem_engines __rcu *engines; struct mutex engines_mutex; /* guards writes to engines */ - struct intel_timeline *timeline; + /** + * @syncobj: Shared timeline syncobj + * + * When the SHARED_TIMELINE flag is set on context creation, this + * provides automatic implicit synchronization across all engines. + */ + struct drm_syncobj *syncobj; /** * @vm: unique address space (GTT) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 96403130a373d..2c56796f6a71b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -3295,6 +3295,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_vma; } + if (eb.gem_context->syncobj) { + struct dma_fence *fence; + + fence = drm_syncobj_fence_get(eb.gem_context->syncobj); + err = i915_request_await_dma_fence(eb.request, fence); + if (err) + goto err_ext; + } + if (in_fence) { if (args->flags & I915_EXEC_FENCE_SUBMIT) err = i915_request_await_execution(eb.request, @@ -3351,6 +3360,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, fput(out_fence->file); } } + + if (eb.gem_context->syncobj) { + drm_syncobj_replace_fence(eb.gem_context->syncobj, + &eb.request->fence); + } + i915_request_put(eb.request); err_vma: -- 2.29.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jason Ekstrand <jason@jlekstrand.net> To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 4/4] drm/i915: Implement SINGLE_TIMELINE with a syncobj Date: Fri, 19 Mar 2021 17:38:56 -0500 [thread overview] Message-ID: <20210319223856.2983244-5-jason@jlekstrand.net> (raw) In-Reply-To: <20210319223856.2983244-1-jason@jlekstrand.net> I'd love to delete the SINGLE_TIMELINE API because it leaks an implementation detail of contexts through to the API and is something that userspace can do itself, trivially. Unfortunately, it's used by the media driver so we can't do that. We can, however, do the next-best thing which is to embed a syncobj in the context and do exactly what we'd expect from userspace internally. This has a couple of advantages. One is that we're no longer leaking a detail of the current execlist scheduler which will be problematic when we try to add GuC scheduling. Second is that, together with deleting the CLONE_CONTEXT API, we should now have a 1:1 mapping between intel_context and intel_timeline which should make some of our locking mess a bit easier. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 47 ++++--------------- .../gpu/drm/i915/gem/i915_gem_context_types.h | 8 +++- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 15 ++++++ 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index f88bac19333ec..e094f4a1ca4cd 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -67,6 +67,8 @@ #include <linux/log2.h> #include <linux/nospec.h> +#include <drm/drm_syncobj.h> + #include "gt/gen6_ppgtt.h" #include "gt/intel_context.h" #include "gt/intel_engine_heartbeat.h" @@ -224,10 +226,6 @@ static void intel_context_set_gem(struct intel_context *ce, ce->vm = vm; } - GEM_BUG_ON(ce->timeline); - if (ctx->timeline) - ce->timeline = intel_timeline_get(ctx->timeline); - if (ctx->sched.priority >= I915_PRIORITY_NORMAL && intel_engine_has_timeslices(ce->engine)) __set_bit(CONTEXT_USE_SEMAPHORES, &ce->flags); @@ -344,8 +342,8 @@ void i915_gem_context_release(struct kref *ref) mutex_destroy(&ctx->engines_mutex); mutex_destroy(&ctx->lut_mutex); - if (ctx->timeline) - intel_timeline_put(ctx->timeline); + if (ctx->syncobj) + drm_syncobj_put(ctx->syncobj); put_pid(ctx->pid); mutex_destroy(&ctx->mutex); @@ -790,33 +788,11 @@ static void __assign_ppgtt(struct i915_gem_context *ctx, i915_vm_close(vm); } -static void __set_timeline(struct intel_timeline **dst, - struct intel_timeline *src) -{ - struct intel_timeline *old = *dst; - - *dst = src ? intel_timeline_get(src) : NULL; - - if (old) - intel_timeline_put(old); -} - -static void __apply_timeline(struct intel_context *ce, void *timeline) -{ - __set_timeline(&ce->timeline, timeline); -} - -static void __assign_timeline(struct i915_gem_context *ctx, - struct intel_timeline *timeline) -{ - __set_timeline(&ctx->timeline, timeline); - context_apply_all(ctx, __apply_timeline, timeline); -} - static struct i915_gem_context * i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) { struct i915_gem_context *ctx; + int ret; if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE && !HAS_EXECLISTS(i915)) @@ -845,16 +821,13 @@ i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) } if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE) { - struct intel_timeline *timeline; - - timeline = intel_timeline_create(&i915->gt); - if (IS_ERR(timeline)) { + ret = drm_syncobj_create(&ctx->syncobj, + DRM_SYNCOBJ_CREATE_SIGNALED, + NULL); + if (ret) { context_close(ctx); - return ERR_CAST(timeline); + return ERR_PTR(ret); } - - __assign_timeline(ctx, timeline); - intel_timeline_put(timeline); } trace_i915_context_create(ctx); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index 676592e27e7d2..8a5fdd163b79d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -83,7 +83,13 @@ struct i915_gem_context { struct i915_gem_engines __rcu *engines; struct mutex engines_mutex; /* guards writes to engines */ - struct intel_timeline *timeline; + /** + * @syncobj: Shared timeline syncobj + * + * When the SHARED_TIMELINE flag is set on context creation, this + * provides automatic implicit synchronization across all engines. + */ + struct drm_syncobj *syncobj; /** * @vm: unique address space (GTT) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 96403130a373d..2c56796f6a71b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -3295,6 +3295,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_vma; } + if (eb.gem_context->syncobj) { + struct dma_fence *fence; + + fence = drm_syncobj_fence_get(eb.gem_context->syncobj); + err = i915_request_await_dma_fence(eb.request, fence); + if (err) + goto err_ext; + } + if (in_fence) { if (args->flags & I915_EXEC_FENCE_SUBMIT) err = i915_request_await_execution(eb.request, @@ -3351,6 +3360,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, fput(out_fence->file); } } + + if (eb.gem_context->syncobj) { + drm_syncobj_replace_fence(eb.gem_context->syncobj, + &eb.request->fence); + } + i915_request_put(eb.request); err_vma: -- 2.29.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-03-19 22:39 UTC|newest] Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-19 22:38 [PATCH 0/4] drm/i915: uAPI clean-ups part 2 Jason Ekstrand 2021-03-19 22:38 ` [Intel-gfx] " Jason Ekstrand 2021-03-19 22:38 ` [PATCH 1/4] drm/i915: Drop I915_CONTEXT_PARAM_RINGSIZE Jason Ekstrand 2021-03-19 22:38 ` [Intel-gfx] " Jason Ekstrand 2021-03-20 14:48 ` Jason Ekstrand 2021-03-20 14:48 ` [Intel-gfx] " Jason Ekstrand 2021-03-22 10:52 ` Matthew Auld 2021-03-22 10:52 ` Matthew Auld 2021-03-22 16:00 ` Jason Ekstrand 2021-03-22 16:00 ` Jason Ekstrand 2021-03-22 12:01 ` Jani Nikula 2021-03-22 12:01 ` Jani Nikula 2021-03-22 16:01 ` Jason Ekstrand 2021-03-22 16:01 ` Jason Ekstrand 2021-03-22 16:26 ` Daniel Vetter 2021-03-22 16:26 ` Daniel Vetter 2021-03-19 22:38 ` [PATCH 2/4] drm/i915: Drop I915_CONTEXT_PARAM_NO_ZEROMAP Jason Ekstrand 2021-03-19 22:38 ` [Intel-gfx] " Jason Ekstrand 2021-03-22 13:00 ` [drm/i915] 014c1518e8: assertion_failure kernel test robot 2021-03-22 13:00 ` kernel test robot 2021-03-22 13:00 ` [Intel-gfx] " kernel test robot 2021-03-22 13:00 ` kernel test robot 2021-03-19 22:38 ` [PATCH 3/4] drm/i915: Drop the CONTEXT_CLONE API Jason Ekstrand 2021-03-19 22:38 ` [Intel-gfx] " Jason Ekstrand 2021-03-22 11:22 ` Tvrtko Ursulin 2021-03-22 11:22 ` Tvrtko Ursulin 2021-03-22 14:09 ` Daniel Vetter 2021-03-22 14:09 ` Daniel Vetter 2021-03-22 14:32 ` Tvrtko Ursulin 2021-03-22 14:32 ` Tvrtko Ursulin 2021-03-22 14:57 ` Daniel Vetter 2021-03-22 14:57 ` Daniel Vetter 2021-03-22 15:31 ` Tvrtko Ursulin 2021-03-22 15:31 ` Tvrtko Ursulin 2021-03-22 16:24 ` Jason Ekstrand 2021-03-22 16:24 ` Jason Ekstrand 2021-03-23 9:46 ` Tvrtko Ursulin 2021-03-23 9:46 ` Tvrtko Ursulin 2021-03-22 16:43 ` Daniel Vetter 2021-03-22 16:43 ` Daniel Vetter 2021-03-23 9:14 ` Tvrtko Ursulin 2021-03-23 9:14 ` Tvrtko Ursulin 2021-03-23 13:23 ` Daniel Vetter 2021-03-23 13:23 ` Daniel Vetter 2021-03-23 16:23 ` Tvrtko Ursulin 2021-03-23 16:23 ` Tvrtko Ursulin 2021-03-23 17:50 ` Jason Ekstrand 2021-03-23 17:50 ` Jason Ekstrand 2021-03-19 22:38 ` Jason Ekstrand [this message] 2021-03-19 22:38 ` [Intel-gfx] [PATCH 4/4] drm/i915: Implement SINGLE_TIMELINE with a syncobj Jason Ekstrand 2021-03-22 12:28 ` Tvrtko Ursulin 2021-03-22 12:28 ` Tvrtko Ursulin 2021-03-22 16:10 ` Jason Ekstrand 2021-03-22 16:10 ` Jason Ekstrand 2021-03-23 9:35 ` Tvrtko Ursulin 2021-03-23 9:35 ` Tvrtko Ursulin 2021-03-23 17:44 ` Jason Ekstrand 2021-03-23 17:44 ` Jason Ekstrand 2021-03-22 16:59 ` Daniel Vetter 2021-03-22 16:59 ` Daniel Vetter 2021-03-22 19:12 ` Jason Ekstrand 2021-03-22 19:12 ` Jason Ekstrand 2021-03-23 17:51 ` [PATCH] drm/i915: Implement SINGLE_TIMELINE with a syncobj (v2) Jason Ekstrand 2021-03-23 17:51 ` [Intel-gfx] " Jason Ekstrand 2021-03-24 9:28 ` Tvrtko Ursulin 2021-03-24 9:28 ` Tvrtko Ursulin 2021-03-24 9:52 ` Daniel Vetter 2021-03-24 9:52 ` Daniel Vetter 2021-03-24 11:36 ` Tvrtko Ursulin 2021-03-24 11:36 ` Tvrtko Ursulin 2021-03-24 17:18 ` Jason Ekstrand 2021-03-24 17:18 ` Jason Ekstrand 2021-03-25 9:48 ` Tvrtko Ursulin 2021-03-25 9:48 ` Tvrtko Ursulin 2021-03-25 9:54 ` Daniel Vetter 2021-03-25 9:54 ` Daniel Vetter 2021-03-24 9:46 ` Daniel Vetter 2021-03-24 9:46 ` Daniel Vetter 2021-03-25 21:13 ` Matthew Brost 2021-03-25 21:13 ` [Intel-gfx] " Matthew Brost 2021-03-25 22:19 ` Jason Ekstrand 2021-03-25 22:19 ` [Intel-gfx] " Jason Ekstrand 2021-03-25 22:21 ` [PATCH 4/4] drm/i915: Implement SINGLE_TIMELINE with a syncobj (v3) Jason Ekstrand 2021-03-25 22:21 ` [Intel-gfx] " Jason Ekstrand 2021-03-19 23:14 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: uAPI clean-ups part 2 Patchwork 2021-03-22 11:55 ` Jani Nikula 2021-03-22 16:11 ` Jason Ekstrand 2021-03-23 21:32 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: uAPI clean-ups part 2 (rev2) Patchwork 2021-03-26 3:01 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: uAPI clean-ups part 2 (rev3) Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210319223856.2983244-5-jason@jlekstrand.net \ --to=jason@jlekstrand.net \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=matthew.brost@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.