* [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline @ 2017-04-10 8:55 Chris Wilson 2017-04-10 8:55 ` [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence Chris Wilson ` (5 more replies) 0 siblings, 6 replies; 8+ messages in thread From: Chris Wilson @ 2017-04-10 8:55 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter 2 clflushes on two different objects are not ordered, and so do not belong to the same timeline (context). Either we use a unique context for each, or we reserve a special global context to mean unordered. Ideally, we would reserve 0 to mean unordered (DMA_FENCE_NO_CONTEXT) to have the same semantics everywhere. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_gem_clflush.c | 8 +------- drivers/gpu/drm/i915/i915_gem_clflush.h | 1 - 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bb6fc1e08a52..b47aae031eef 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1511,6 +1511,8 @@ struct i915_gem_mm { /** LRU list of objects with fence regs on them. */ struct list_head fence_list; + u64 unordered_timeline; + /* the indicator for dispatch video commands on two BSD rings */ atomic_t bsd_engine_dispatch_index; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b210acc3d0b4..79b5eab5ae83 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4681,7 +4681,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) mutex_lock(&dev_priv->drm.struct_mutex); - i915_gem_clflush_init(dev_priv); + dev_priv->mm.unordered_timeline = dma_fence_context_alloc(1); if (!i915.enable_execlists) { dev_priv->gt.resume = intel_legacy_submission_resume; diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c index ffd01e02fe94..ffac7a1f0caf 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c @@ -27,7 +27,6 @@ #include "i915_gem_clflush.h" static DEFINE_SPINLOCK(clflush_lock); -static u64 clflush_context; struct clflush { struct dma_fence dma; /* Must be first for dma_fence_free() */ @@ -157,7 +156,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, dma_fence_init(&clflush->dma, &i915_clflush_ops, &clflush_lock, - clflush_context, + to_i915(obj->base.dev)->mm.unordered_timeline, 0); i915_sw_fence_init(&clflush->wait, i915_clflush_notify); @@ -182,8 +181,3 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, GEM_BUG_ON(obj->base.write_domain != I915_GEM_DOMAIN_CPU); } } - -void i915_gem_clflush_init(struct drm_i915_private *i915) -{ - clflush_context = dma_fence_context_alloc(1); -} diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.h b/drivers/gpu/drm/i915/i915_gem_clflush.h index b62d61a2d15f..2455a7820937 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.h +++ b/drivers/gpu/drm/i915/i915_gem_clflush.h @@ -28,7 +28,6 @@ struct drm_i915_private; struct drm_i915_gem_object; -void i915_gem_clflush_init(struct drm_i915_private *i915); void i915_gem_clflush_object(struct drm_i915_gem_object *obj, unsigned int flags); #define I915_CLFLUSH_FORCE BIT(0) -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson @ 2017-04-10 8:55 ` Chris Wilson 2017-04-10 10:21 ` Joonas Lahtinen 2017-04-10 8:55 ` [PATCH 3/5] drm/i915: Make ptr_unpack_bits() more function-like Chris Wilson ` (4 subsequent siblings) 5 siblings, 1 reply; 8+ messages in thread From: Chris Wilson @ 2017-04-10 8:55 UTC (permalink / raw) To: intel-gfx Currently we filter out repeated use of the same timeline in the low level i915_gem_request_await_request(), after having added the dependency on the old request. However, we can lift this to i915_gem_request_await_dma_fence() (before the dependency is added) using the observation that requests along the same timeline are explicitly ordered via i915_add_request (along with the dependencies). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem_request.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 313cdff7c6dd..31874a38752e 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -663,6 +663,7 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to, int ret; GEM_BUG_ON(to == from); + GEM_BUG_ON(to->timeline == from->timeline); if (to->engine->schedule) { ret = i915_priotree_add_dependency(to->i915, @@ -672,9 +673,6 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to, return ret; } - if (to->timeline == from->timeline) - return 0; - if (to->engine == from->engine) { ret = i915_sw_fence_await_sw_fence_gfp(&to->submit, &from->submit, @@ -723,6 +721,13 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return 0; + /* Requests on the same timeline are explicitly ordered, along with + * their dependencies, by i915_add_request() which ensures that requests + * are submitted in-order through each ring. + */ + if (fence->context == req->fence.context) + return 0; + if (dma_fence_is_i915(fence)) return i915_gem_request_await_request(req, to_request(fence)); -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence 2017-04-10 8:55 ` [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence Chris Wilson @ 2017-04-10 10:21 ` Joonas Lahtinen 0 siblings, 0 replies; 8+ messages in thread From: Joonas Lahtinen @ 2017-04-10 10:21 UTC (permalink / raw) To: Chris Wilson, intel-gfx On ma, 2017-04-10 at 09:55 +0100, Chris Wilson wrote: > Currently we filter out repeated use of the same timeline in the low > level i915_gem_request_await_request(), after having added the > dependency on the old request. However, we can lift this to > i915_gem_request_await_dma_fence() (before the dependency is added) > using the observation that requests along the same timeline are > explicitly ordered via i915_add_request (along with the dependencies). > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/5] drm/i915: Make ptr_unpack_bits() more function-like 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson 2017-04-10 8:55 ` [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence Chris Wilson @ 2017-04-10 8:55 ` Chris Wilson 2017-04-10 8:55 ` [PATCH 4/5] drm/i915: Redefine ptr_pack_bits() and friends Chris Wilson ` (3 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2017-04-10 8:55 UTC (permalink / raw) To: intel-gfx ptr_unpack_bits() is a function-like macro, as such it is meant to be replaceable by a function. In this case, we should be passing in the out-param as a pointer. Bizarrely this does affect code generation: function old new delta i915_gem_object_pin_map 409 389 -20 An improvement(?) in this case, but one can't help wonder what strict-aliasing optimisations we are preventing. The generated code looks identical in using ptr_unpack_bits (no extra motions to stack, the pointer and bits appear to be kept in registers), the difference appears to be code ordering and with a reorder it is able to use smaller forward jumps. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_utils.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 79b5eab5ae83..f39b36a8622a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2560,7 +2560,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, } GEM_BUG_ON(!obj->mm.pages); - ptr = ptr_unpack_bits(obj->mm.mapping, has_type); + ptr = ptr_unpack_bits(obj->mm.mapping, &has_type); if (ptr && has_type != type) { if (pinned) { ret = -EBUSY; diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index c5455d36b617..aca11aad5da7 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -77,7 +77,7 @@ #define ptr_unpack_bits(ptr, bits) ({ \ unsigned long __v = (unsigned long)(ptr); \ - (bits) = __v & ~PAGE_MASK; \ + *(bits) = __v & ~PAGE_MASK; \ (typeof(ptr))(__v & PAGE_MASK); \ }) -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/5] drm/i915: Redefine ptr_pack_bits() and friends 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson 2017-04-10 8:55 ` [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence Chris Wilson 2017-04-10 8:55 ` [PATCH 3/5] drm/i915: Make ptr_unpack_bits() more function-like Chris Wilson @ 2017-04-10 8:55 ` Chris Wilson 2017-04-10 8:55 ` [PATCH 5/5] drm/i915: Squash repeated awaits on the same fence Chris Wilson ` (2 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2017-04-10 8:55 UTC (permalink / raw) To: intel-gfx Rebrand the current (pointer | bits) pack/unpack utility macros as explicit bit twiddling for PAGE_SIZE so that we can use the more flexible underlying macros for different bits. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_cmd_parser.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 6 +++--- drivers/gpu/drm/i915/i915_utils.h | 19 +++++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 7af100f84410..1b69a1f9a1ea 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -1284,7 +1284,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine, if (*cmd == MI_BATCH_BUFFER_END) { if (needs_clflush_after) { - void *ptr = ptr_mask_bits(shadow_batch_obj->mm.mapping); + void *ptr = page_mask_bits(shadow_batch_obj->mm.mapping); drm_clflush_virt_range(ptr, (void *)(cmd + 1) - ptr); } diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f39b36a8622a..3c0e63234021 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2228,7 +2228,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, if (obj->mm.mapping) { void *ptr; - ptr = ptr_mask_bits(obj->mm.mapping); + ptr = page_mask_bits(obj->mm.mapping); if (is_vmalloc_addr(ptr)) vunmap(ptr); else @@ -2560,7 +2560,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, } GEM_BUG_ON(!obj->mm.pages); - ptr = ptr_unpack_bits(obj->mm.mapping, &has_type); + ptr = page_unpack_bits(obj->mm.mapping, &has_type); if (ptr && has_type != type) { if (pinned) { ret = -EBUSY; @@ -2582,7 +2582,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, goto err_unpin; } - obj->mm.mapping = ptr_pack_bits(ptr, type); + obj->mm.mapping = page_pack_bits(ptr, type); } out_unlock: diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index aca11aad5da7..f0500c65726d 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -70,20 +70,27 @@ #define overflows_type(x, T) \ (sizeof(x) > sizeof(T) && (x) >> (sizeof(T) * BITS_PER_BYTE)) -#define ptr_mask_bits(ptr) ({ \ +#define ptr_mask_bits(ptr, n) ({ \ unsigned long __v = (unsigned long)(ptr); \ - (typeof(ptr))(__v & PAGE_MASK); \ + (typeof(ptr))(__v & -BIT(n)); \ }) -#define ptr_unpack_bits(ptr, bits) ({ \ +#define ptr_unmask_bits(ptr, n) ((unsigned long)(ptr) & (BIT(n) - 1)) + +#define ptr_unpack_bits(ptr, bits, n) ({ \ unsigned long __v = (unsigned long)(ptr); \ - *(bits) = __v & ~PAGE_MASK; \ - (typeof(ptr))(__v & PAGE_MASK); \ + *(bits) = __v & (BIT(n) - 1); \ + (typeof(ptr))(__v & -BIT(n)); \ }) -#define ptr_pack_bits(ptr, bits) \ +#define ptr_pack_bits(ptr, bits, n) \ ((typeof(ptr))((unsigned long)(ptr) | (bits))) +#define page_mask_bits(ptr) ptr_mask_bits(ptr, PAGE_SHIFT) +#define page_unmask_bits(ptr) ptr_unmask_bits(ptr, PAGE_SHIFT) +#define page_pack_bits(ptr, bits) ptr_pack_bits(ptr, bits, PAGE_SHIFT) +#define page_unpack_bits(ptr, bits) ptr_unpack_bits(ptr, bits, PAGE_SHIFT) + #define ptr_offset(ptr, member) offsetof(typeof(*(ptr)), member) #define fetch_and_zero(ptr) ({ \ -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/5] drm/i915: Squash repeated awaits on the same fence 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson ` (2 preceding siblings ...) 2017-04-10 8:55 ` [PATCH 4/5] drm/i915: Redefine ptr_pack_bits() and friends Chris Wilson @ 2017-04-10 8:55 ` Chris Wilson 2017-04-10 9:27 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Patchwork 2017-04-10 10:17 ` [PATCH 1/5] " Joonas Lahtinen 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2017-04-10 8:55 UTC (permalink / raw) To: intel-gfx Track the latest fence waited upon on each context, and only add a new asynchronous wait if the new fence is more recent than the recorded fence for that context. This requires us to filter out unordered timelines, which are noted by DMA_FENCE_NO_CONTEXT. However, in the absence of a universal identifier, we have to use our own i915->mm.unordered_timeline token. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem_request.c | 72 +++--- drivers/gpu/drm/i915/i915_gem_timeline.c | 255 +++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_timeline.h | 23 ++ drivers/gpu/drm/i915/selftests/i915_gem_timeline.c | 127 ++++++++++ .../gpu/drm/i915/selftests/i915_mock_selftests.h | 1 + lib/radix-tree.c | 1 + 6 files changed, 452 insertions(+), 27 deletions(-) create mode 100644 drivers/gpu/drm/i915/selftests/i915_gem_timeline.c diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 31874a38752e..6e68387aa097 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -714,9 +714,7 @@ int i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, struct dma_fence *fence) { - struct dma_fence_array *array; int ret; - int i; if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return 0; @@ -728,39 +726,59 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, if (fence->context == req->fence.context) return 0; - if (dma_fence_is_i915(fence)) - return i915_gem_request_await_request(req, to_request(fence)); + /* Squash repeated waits to the same timelines, picking the latest */ + if (fence->context != req->i915->mm.unordered_timeline) { + if (intel_timeline_sync_get(req->timeline, + fence->context, + fence->seqno)) + return 0; - if (!dma_fence_is_array(fence)) { + ret = intel_timeline_sync_reserve(req->timeline); + if (unlikely(ret)) + return ret; + } + + if (dma_fence_is_i915(fence)) { + ret = i915_gem_request_await_request(req, to_request(fence)); + if (ret < 0) + return ret; + } else if (!dma_fence_is_array(fence)) { ret = i915_sw_fence_await_dma_fence(&req->submit, fence, I915_FENCE_TIMEOUT, GFP_KERNEL); - return ret < 0 ? ret : 0; - } - - /* Note that if the fence-array was created in signal-on-any mode, - * we should *not* decompose it into its individual fences. However, - * we don't currently store which mode the fence-array is operating - * in. Fortunately, the only user of signal-on-any is private to - * amdgpu and we should not see any incoming fence-array from - * sync-file being in signal-on-any mode. - */ - - array = to_dma_fence_array(fence); - for (i = 0; i < array->num_fences; i++) { - struct dma_fence *child = array->fences[i]; - - if (dma_fence_is_i915(child)) - ret = i915_gem_request_await_request(req, - to_request(child)); - else - ret = i915_sw_fence_await_dma_fence(&req->submit, - child, I915_FENCE_TIMEOUT, - GFP_KERNEL); if (ret < 0) return ret; + } else { + struct dma_fence_array *array = to_dma_fence_array(fence); + int i; + + /* Note that if the fence-array was created in signal-on-any mode, + * we should *not* decompose it into its individual fences. However, + * we don't currently store which mode the fence-array is operating + * in. Fortunately, the only user of signal-on-any is private to + * amdgpu and we should not see any incoming fence-array from + * sync-file being in signal-on-any mode. + */ + + for (i = 0; i < array->num_fences; i++) { + struct dma_fence *child = array->fences[i]; + + if (dma_fence_is_i915(child)) + ret = i915_gem_request_await_request(req, + to_request(child)); + else + ret = i915_sw_fence_await_dma_fence(&req->submit, + child, I915_FENCE_TIMEOUT, + GFP_KERNEL); + if (ret < 0) + return ret; + } } + if (fence->context != req->i915->mm.unordered_timeline) + intel_timeline_sync_set(req->timeline, + fence->context, fence->seqno); + return 0; } diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.c b/drivers/gpu/drm/i915/i915_gem_timeline.c index b596ca7ee058..30b4d07a5444 100644 --- a/drivers/gpu/drm/i915/i915_gem_timeline.c +++ b/drivers/gpu/drm/i915/i915_gem_timeline.c @@ -24,6 +24,255 @@ #include "i915_drv.h" +#if 0 +#define assert(x) BUG_ON(!(x)) +#define dbg(x) pr_err x +#else +#define assert(x) do { } while (0) +#define dbg(x) do { } while (0) +#endif + +#define SHIFT ilog2(NSEQMAP) +#define MASK (NSEQMAP - 1) + +static void seqmap_free_layers(struct seqmap_layer *p) +{ + unsigned int i; + + if (p->height) { + for (; (i = ffs(p->bitmap)); p->bitmap &= ~0u << i) + seqmap_free_layers(p->slot[i - 1]); + } + + kfree(p); +} + +static void seqmap_free(struct seqmap *seqmap) +{ + if (seqmap->top) + seqmap_free_layers(seqmap->top); + + while (seqmap->freed) { + struct seqmap_layer *p; + + p = ptr_mask_bits(seqmap->freed, 2); + seqmap->freed = p->parent; + kfree(p); + } +} + +__malloc static struct seqmap_layer * +seqmap_alloc_layer(struct seqmap *shared) +{ + struct seqmap_layer *p; + + assert(shared->freed); + + p = ptr_mask_bits(shared->freed, 2); + shared->freed = p->parent; + + return p; +} + +static int layer_idx(const struct seqmap_layer *p, u64 id) +{ + return (id >> p->height) & MASK; +} + +bool intel_timeline_sync_get(struct intel_timeline *tl, u64 id, u32 seqno) +{ + struct seqmap *shared = &tl->sync; + struct seqmap_layer *p; + unsigned int idx; + + dbg(("%s(id=%llx, top? %d)\n", __func__, id, !!shared->top)); + + if (!shared->top) + return false; + + /* First see if this fence is in the same layer as the previous fence */ + p = shared->hint; + if ((id >> SHIFT) == p->prefix) + goto found; + + p = shared->top; + do { + dbg(("id=%llx, p->(prefix=%llx, height=%d), delta=%llx, idx=%x\n", + id, p->prefix, p->height, + id >> p->height >> SHIFT, + layer_idx(p, id))); + + if ((id >> p->height >> SHIFT) != p->prefix) + return false; + + if (!p->height) /* matching base layer */ + break; + + /* descend into the next layer */ + p = p->slot[layer_idx(p, id)]; + if (unlikely(!p)) + return false; + } while (1); + + shared->hint = p; +found: + idx = id & MASK; + dbg(("id=%llx, found layer [idx=%u], present? %lu\n", + id, idx, p->bitmap & BIT(idx))); + if (!(p->bitmap & BIT(idx))) + return false; + + return i915_seqno_passed((uintptr_t)p->slot[idx], seqno); +} + +void intel_timeline_sync_set(struct intel_timeline *tl, u64 id, u32 seqno) +{ + struct seqmap *shared = &tl->sync; + struct seqmap_layer *p, *cur; + unsigned int idx; + + dbg(("%s(id=%llx)\n", __func__, id)); + + /* First see if this fence is in the same layer as the previous fence */ + p = shared->hint; + if (p && (id >> SHIFT) == p->prefix) { + assert(p->height == 0); + goto found_layer; + } + + p = shared->top; + if (!p) { + cur = seqmap_alloc_layer(shared); + cur->parent = NULL; + shared->top = cur; + goto new_layer; + } + + /* No shortcut, we have to descend the tree to find the right layer + * containing this fence. + * + * Each layer in the tree holds 16 (NSHARED) pointers, either fences + * or lower layers. Leaf nodes (height = 0) contain the fences, all + * other nodes (height > 0) are internal layers that point to a lower + * node. Each internal layer has at least 2 descendents. + * + * Starting at the top, we check whether the current prefix matches. If + * it doesn't, we have gone passed our layer and need to insert a join + * into the tree, and a new leaf node as a descendent as well as the + * original layer. + * + * The matching prefix means we are still following the right branch + * of the tree. If it has height 0, we have found our leaf and just + * need to replace the fence slot with ourselves. If the height is + * not zero, our slot contains the next layer in the tree (unless + * it is empty, in which case we can add ourselves as a new leaf). + * As descend the tree the prefix grows (and height decreases). + */ + do { + dbg(("id=%llx, p->(prefix=%llx, height=%d), delta=%llx, idx=%x\n", + id, p->prefix, p->height, + id >> p->height >> SHIFT, + layer_idx(p, id))); + + if ((id >> p->height >> SHIFT) != p->prefix) { + /* insert a join above the current layer */ + cur = seqmap_alloc_layer(shared); + cur->height = ALIGN(fls64((id >> p->height >> SHIFT) ^ p->prefix), + SHIFT) + p->height; + cur->prefix = id >> cur->height >> SHIFT; + + dbg(("id=%llx, join prefix=%llu, height=%d\n", + id, cur->prefix, cur->height)); + + assert((id >> cur->height >> SHIFT) == cur->prefix); + assert(cur->height > p->height); + + if (p->parent) { + assert(p->parent->slot[layer_idx(p->parent, id)] == p); + assert(cur->height < p->parent->height); + p->parent->slot[layer_idx(p->parent, id)] = cur; + } else { + shared->top = cur; + } + cur->parent = p->parent; + + idx = p->prefix >> (cur->height - p->height - SHIFT) & MASK; + cur->slot[idx] = p; + cur->bitmap |= BIT(idx); + p->parent = cur; + } else if (!p->height) { + /* matching base layer */ + shared->hint = p; + goto found_layer; + } else { + /* descend into the next layer */ + idx = layer_idx(p, id); + cur = p->slot[idx]; + if (unlikely(!cur)) { + cur = seqmap_alloc_layer(shared); + p->slot[idx] = cur; + p->bitmap |= BIT(idx); + cur->parent = p; + goto new_layer; + } + } + + p = cur; + } while (1); + +found_layer: + assert(p->height == 0); + idx = id & MASK; + p->slot[idx] = (void *)(uintptr_t)seqno; + p->bitmap |= BIT(idx); + dbg(("id=%llx, found existing layer prefix=%llx, idx=%x [bitmap=%x]\n", + id, p->prefix, idx, p->bitmap)); + return; + +new_layer: + assert(cur->height == 0); + assert(cur->bitmap == 0); + idx = id & MASK; + cur->prefix = id >> SHIFT; + cur->slot[idx] = (void *)(uintptr_t)seqno; + cur->bitmap = BIT(idx); + shared->hint = cur; + dbg(("id=%llx, new layer prefix=%llx, idx=%x [bitmap=%x]\n", + id, cur->prefix, (int)(id & MASK), cur->bitmap)); + return; +} + +int intel_timeline_sync_reserve(struct intel_timeline *tl) +{ + struct seqmap *shared = &tl->sync; + int count; + + might_sleep(); + + /* To guarantee being able to replace a fence in the radixtree, + * we need at most 2 layers: one to create a join in the tree, + * and one to contain the fence. Typically we expect to reuse + * a layer and so avoid any insertions. + * + * We use the low bits of the freed list to track its length + * since we only need a couple of bits. + */ + count = ptr_unmask_bits(shared->freed, 2); + while (count++ < 2) { + struct seqmap_layer *p; + + p = kzalloc(sizeof(*p), GFP_KERNEL); + if (unlikely(!p)) + return -ENOMEM; + + p->parent = shared->freed; + shared->freed = ptr_pack_bits(p, count, 2); + } + + return 0; +} + + static int __i915_gem_timeline_init(struct drm_i915_private *i915, struct i915_gem_timeline *timeline, const char *name, @@ -91,8 +340,14 @@ void i915_gem_timeline_fini(struct i915_gem_timeline *timeline) struct intel_timeline *tl = &timeline->engine[i]; GEM_BUG_ON(!list_empty(&tl->requests)); + + seqmap_free(&tl->sync); } list_del(&timeline->link); kfree(timeline->name); } + +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) +#include "selftests/i915_gem_timeline.c" +#endif diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.h b/drivers/gpu/drm/i915/i915_gem_timeline.h index 6c53e14cab2a..5d0e37f212ac 100644 --- a/drivers/gpu/drm/i915/i915_gem_timeline.h +++ b/drivers/gpu/drm/i915/i915_gem_timeline.h @@ -26,11 +26,28 @@ #define I915_GEM_TIMELINE_H #include <linux/list.h> +#include <linux/radix-tree.h> #include "i915_gem_request.h" struct i915_gem_timeline; +#define NSEQMAP 16 + +struct seqmap_layer { + u64 prefix; + unsigned int height; + unsigned int bitmap; + struct seqmap_layer *parent; + void *slot[NSEQMAP]; +}; + +struct seqmap { + struct seqmap_layer *hint; + struct seqmap_layer *top; + struct seqmap_layer *freed; +}; + struct intel_timeline { u64 fence_context; u32 seqno; @@ -55,6 +72,8 @@ struct intel_timeline { * struct_mutex. */ struct i915_gem_active last_request; + + struct seqmap sync; u32 sync_seqno[I915_NUM_ENGINES]; struct i915_gem_timeline *common; @@ -75,4 +94,8 @@ int i915_gem_timeline_init(struct drm_i915_private *i915, int i915_gem_timeline_init__global(struct drm_i915_private *i915); void i915_gem_timeline_fini(struct i915_gem_timeline *tl); +bool intel_timeline_sync_get(struct intel_timeline *tl, u64 id, u32 seqno); +int intel_timeline_sync_reserve(struct intel_timeline *tl); +void intel_timeline_sync_set(struct intel_timeline *tl, u64 id, u32 seqno); + #endif diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_timeline.c b/drivers/gpu/drm/i915/selftests/i915_gem_timeline.c new file mode 100644 index 000000000000..15e2b2e63081 --- /dev/null +++ b/drivers/gpu/drm/i915/selftests/i915_gem_timeline.c @@ -0,0 +1,127 @@ +/* + * Copyright © 2017 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "../i915_selftest.h" +#include "mock_gem_device.h" + +static int igt_seqmap(void *arg) +{ + struct drm_i915_private *i915 = arg; + const struct { + const char *name; + u32 seqno; + bool expected; + bool set; + } pass[] = { + { "unset", 0, false, false }, + { "new", 0, false, true }, + { "0a", 0, true, true }, + { "1a", 1, false, true }, + { "1b", 1, true, true }, + { "0b", 0, true, false }, + { "2a", 2, false, true }, + { "4", 4, false, true }, + { "INT_MAX", INT_MAX, false, true }, + { "INT_MAX-1", INT_MAX-1, true, false }, + { "INT_MAX+1", (u32)INT_MAX+1, false, true }, + { "INT_MAX", INT_MAX, true, false }, + { "UINT_MAX", UINT_MAX, false, true }, + { "wrap", 0, false, true }, + { "unwrap", UINT_MAX, true, false }, + {}, + }, *p; + struct intel_timeline *tl; + int order, offset; + int ret; + + tl = &i915->gt.global_timeline.engine[RCS]; + for (p = pass; p->name; p++) { + for (order = 1; order < 64; order++) { + for (offset = -1; offset <= (order > 1); offset++) { + u64 ctx = BIT_ULL(order) + offset; + + if (intel_timeline_sync_get(tl, + ctx, + p->seqno) != p->expected) { + pr_err("1: %s(ctx=%llu, seqno=%u) expected passed %s but failed\n", + p->name, ctx, p->seqno, yesno(p->expected)); + return -EINVAL; + } + + if (p->set) { + ret = intel_timeline_sync_reserve(tl); + if (ret) + return ret; + + intel_timeline_sync_set(tl, ctx, p->seqno); + } + } + } + } + + tl = &i915->gt.global_timeline.engine[BCS]; + for (order = 1; order < 64; order++) { + for (offset = -1; offset <= (order > 1); offset++) { + u64 ctx = BIT_ULL(order) + offset; + + for (p = pass; p->name; p++) { + if (intel_timeline_sync_get(tl, + ctx, + p->seqno) != p->expected) { + pr_err("2: %s(ctx=%llu, seqno=%u) expected passed %s but failed\n", + p->name, ctx, p->seqno, yesno(p->expected)); + return -EINVAL; + } + + if (p->set) { + ret = intel_timeline_sync_reserve(tl); + if (ret) + return ret; + + intel_timeline_sync_set(tl, ctx, p->seqno); + } + } + } + } + + return 0; +} + +int i915_gem_timeline_mock_selftests(void) +{ + static const struct i915_subtest tests[] = { + SUBTEST(igt_seqmap), + }; + struct drm_i915_private *i915; + int err; + + i915 = mock_gem_device(); + if (!i915) + return -ENOMEM; + + err = i915_subtests(tests, i915); + drm_dev_unref(&i915->drm); + + return err; +} diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h index be9a9ebf5692..8d0f50c25df8 100644 --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h @@ -12,6 +12,7 @@ selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt selfcheck) */ selftest(scatterlist, scatterlist_mock_selftests) selftest(uncore, intel_uncore_mock_selftests) selftest(breadcrumbs, intel_breadcrumbs_mock_selftests) +selftest(timelines, i915_gem_timeline_mock_selftests) selftest(requests, i915_gem_request_mock_selftests) selftest(objects, i915_gem_object_mock_selftests) selftest(dmabuf, i915_gem_dmabuf_mock_selftests) diff --git a/lib/radix-tree.c b/lib/radix-tree.c index 691a9ad48497..84cccf7138c4 100644 --- a/lib/radix-tree.c +++ b/lib/radix-tree.c @@ -2022,6 +2022,7 @@ void radix_tree_iter_delete(struct radix_tree_root *root, if (__radix_tree_delete(root, iter->node, slot)) iter->index = iter->next_index; } +EXPORT_SYMBOL(radix_tree_iter_delete); /** * radix_tree_delete_item - delete an item from a radix tree -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson ` (3 preceding siblings ...) 2017-04-10 8:55 ` [PATCH 5/5] drm/i915: Squash repeated awaits on the same fence Chris Wilson @ 2017-04-10 9:27 ` Patchwork 2017-04-10 10:17 ` [PATCH 1/5] " Joonas Lahtinen 5 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2017-04-10 9:27 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline URL : https://patchwork.freedesktop.org/series/22759/ State : success == Summary == Series 22759v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/22759/revisions/1/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-snb-2600) fdo#100125 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:427s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:429s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:585s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:500s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:539s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:484s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:405s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:420s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:490s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:463s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:457s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:567s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:449s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:572s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:457s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:496s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:433s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:530s fi-snb-2600 total:278 pass:248 dwarn:1 dfail:0 fail:0 skip:29 time:417s d86f5fa557fdf089bdb5b51f625a7f5967f11015 drm-tip: 2017y-04m-10d-06h-38m-49s UTC integration manifest 2788baf drm/i915: Squash repeated awaits on the same fence 926bcad drm/i915: Redefine ptr_pack_bits() and friends 5a4ed59 drm/i915: Make ptr_unpack_bits() more function-like 71fdbdc drm/i915: Lift timeline ordering to await_dma_fence a64db54 drm/i915: Mark up clflushes as belonging to an unordered timeline == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4454/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson ` (4 preceding siblings ...) 2017-04-10 9:27 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Patchwork @ 2017-04-10 10:17 ` Joonas Lahtinen 5 siblings, 0 replies; 8+ messages in thread From: Joonas Lahtinen @ 2017-04-10 10:17 UTC (permalink / raw) To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter On ma, 2017-04-10 at 09:55 +0100, Chris Wilson wrote: > 2 clflushes on two different objects are not ordered, and so do not > belong to the same timeline (context). Either we use a unique context > for each, or we reserve a special global context to mean unordered. > Ideally, we would reserve 0 to mean unordered (DMA_FENCE_NO_CONTEXT) to > have the same semantics everywhere. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> <SNIP> > @@ -157,7 +156,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, > dma_fence_init(&clflush->dma, > &i915_clflush_ops, > &clflush_lock, > - clflush_context, > + to_i915(obj->base.dev)->mm.unordered_timeline, It seems you have high confidence on being able to replace this line with DMA_FENCE_NO_CONTEXT ;) Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-10 10:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-10 8:55 [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Chris Wilson 2017-04-10 8:55 ` [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence Chris Wilson 2017-04-10 10:21 ` Joonas Lahtinen 2017-04-10 8:55 ` [PATCH 3/5] drm/i915: Make ptr_unpack_bits() more function-like Chris Wilson 2017-04-10 8:55 ` [PATCH 4/5] drm/i915: Redefine ptr_pack_bits() and friends Chris Wilson 2017-04-10 8:55 ` [PATCH 5/5] drm/i915: Squash repeated awaits on the same fence Chris Wilson 2017-04-10 9:27 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline Patchwork 2017-04-10 10:17 ` [PATCH 1/5] " Joonas Lahtinen
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.