Re: [Intel-gfx] [PATCH 19/19] drm/i915: Replace struct_mutex for batch pool serialisation
On Mon, 24 Jun 2019 at 06:45, Chris Wilson wrote:
>
> Switch to tracking activity via i915_active on individual nodes, only
> keeping a list of retired objects in the cache, and reaping the cache
> when the engine itself idles.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/Makefile | 2 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 58 ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 1 -
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 1 -
> drivers/gpu/drm/i915/gem/i915_gem_pm.c| 4 +-
> drivers/gpu/drm/i915/gt/intel_engine.h| 1 -
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 +-
> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +
> drivers/gpu/drm/i915/gt/intel_engine_pool.c | 164 ++
> drivers/gpu/drm/i915/gt/intel_engine_pool.h | 34
> .../gpu/drm/i915/gt/intel_engine_pool_types.h | 29
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 6 +-
> drivers/gpu/drm/i915/gt/mock_engine.c | 3 +
> drivers/gpu/drm/i915/i915_debugfs.c | 68
> drivers/gpu/drm/i915/i915_gem_batch_pool.h| 26 ---
> 15 files changed, 277 insertions(+), 133 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool.c
> create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool.h
> create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool_types.h
> delete mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.h
i915_gem_batch_pool.c is still skulking around somewhere :)
[snip]
> +static struct list_head *
> +bucket_for_size(struct intel_engine_pool *pool, size_t sz)
> +{
> + int n;
> +
> + /*
> +* Compute a power-of-two bucket, but throw everything greater than
> +* 16KiB into the same bucket: i.e. the the buckets hold objects of
the buckets
> +* (1 page, 2 pages, 4 pages, 8+ pages).
> +*/
> + n = fls(sz >> PAGE_SHIFT) - 1;
> + if (n >= ARRAY_SIZE(pool->cache_list))
> + n = ARRAY_SIZE(pool->cache_list) - 1;
> +
> + return >cache_list[n];
> +}
> +
> +static void node_free(struct intel_engine_pool_node *node)
> +{
> + i915_gem_object_put(node->obj);
> + kfree(node);
> +}
> +
> +static int pool_active(struct i915_active *ref)
> +{
> + struct intel_engine_pool_node *node =
> + container_of(ref, typeof(*node), active);
> + struct reservation_object *resv = node->obj->base.resv;
> +
> + if (reservation_object_trylock(resv)) {
> + reservation_object_add_excl_fence(resv, NULL);
> + reservation_object_unlock(resv);
> + }
> +
> + return i915_gem_object_pin_pages(node->obj);
> +}
> +
> +static void pool_retire(struct i915_active *ref)
> +{
> + struct intel_engine_pool_node *node =
> + container_of(ref, typeof(*node), active);
> + struct intel_engine_pool *pool = node->pool;
> + struct list_head *list = bucket_for_size(pool, node->obj->base.size);
> + unsigned long flags;
> +
> + GEM_BUG_ON(!intel_engine_pm_is_awake(to_engine(pool)));
> +
> + i915_gem_object_unpin_pages(node->obj);
> +
> + spin_lock_irqsave(>lock, flags);
> + list_add(>link, list);
> + spin_unlock_irqrestore(>lock, flags);
> +}
> +
> +static struct intel_engine_pool_node *
> +node_create(struct intel_engine_pool *pool, size_t sz)
> +{
> + struct intel_engine_cs *engine = to_engine(pool);
> + struct intel_engine_pool_node *node;
> + struct drm_i915_gem_object *obj;
> +
> + node = kmalloc(sizeof(*node),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> + if (!node)
> + return ERR_PTR(-ENOMEM);
> +
> + node->pool = pool;
> + i915_active_init(engine->i915, >active, pool_active,
> pool_retire);
> +
> + obj = i915_gem_object_create_internal(engine->i915, sz);
> + if (IS_ERR(obj)) {
> + kfree(node);
> + return ERR_CAST(obj);
i915_active_fini() somewhere, ditto for node_free?
Anyway, looks good.
Reviewed-by: Matthew Auld
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/19] drm/i915: Replace struct_mutex for batch pool serialisation
Switch to tracking activity via i915_active on individual nodes, only
keeping a list of retired objects in the cache, and reaping the cache
when the engine itself idles.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/Makefile | 2 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 58 ---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 1 -
.../gpu/drm/i915/gem/i915_gem_object_types.h | 1 -
drivers/gpu/drm/i915/gem/i915_gem_pm.c| 4 +-
drivers/gpu/drm/i915/gt/intel_engine.h| 1 -
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 +-
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +
drivers/gpu/drm/i915/gt/intel_engine_pool.c | 164 ++
drivers/gpu/drm/i915/gt/intel_engine_pool.h | 34
.../gpu/drm/i915/gt/intel_engine_pool_types.h | 29
drivers/gpu/drm/i915/gt/intel_engine_types.h | 6 +-
drivers/gpu/drm/i915/gt/mock_engine.c | 3 +
drivers/gpu/drm/i915/i915_debugfs.c | 68
drivers/gpu/drm/i915/i915_gem_batch_pool.h| 26 ---
15 files changed, 277 insertions(+), 133 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool.c
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool.h
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool_types.h
delete mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 84ac0fd1b8d0..ad8b9f1887a0 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -72,6 +72,7 @@ obj-y += gt/
gt-y += \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
+ gt/intel_engine_pool.o \
gt/intel_engine_cs.o \
gt/intel_engine_pm.o \
gt/intel_gt.o \
@@ -118,7 +119,6 @@ i915-y += \
$(gem-y) \
i915_active.o \
i915_cmd_parser.o \
- i915_gem_batch_pool.o \
i915_gem_evict.o \
i915_gem_fence_reg.o \
i915_gem_gtt.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 80c9c57a302f..0ea2d49bc8b9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -16,6 +16,7 @@
#include "gem/i915_gem_ioctls.h"
#include "gt/intel_context.h"
+#include "gt/intel_engine_pool.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
@@ -1145,25 +1146,26 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
unsigned int len)
{
struct reloc_cache *cache = >reloc_cache;
- struct drm_i915_gem_object *obj;
+ struct intel_engine_pool_node *pool;
struct i915_request *rq;
struct i915_vma *batch;
u32 *cmd;
int err;
- obj = i915_gem_batch_pool_get(>engine->batch_pool, PAGE_SIZE);
- if (IS_ERR(obj))
- return PTR_ERR(obj);
+ pool = intel_engine_pool_get(>engine->pool, PAGE_SIZE);
+ if (IS_ERR(pool))
+ return PTR_ERR(pool);
- cmd = i915_gem_object_pin_map(obj,
+ cmd = i915_gem_object_pin_map(pool->obj,
cache->has_llc ?
I915_MAP_FORCE_WB :
I915_MAP_FORCE_WC);
- i915_gem_object_unpin_pages(obj);
- if (IS_ERR(cmd))
- return PTR_ERR(cmd);
+ if (IS_ERR(cmd)) {
+ err = PTR_ERR(cmd);
+ goto out_pool;
+ }
- batch = i915_vma_instance(obj, vma->vm, NULL);
+ batch = i915_vma_instance(pool->obj, vma->vm, NULL);
if (IS_ERR(batch)) {
err = PTR_ERR(batch);
goto err_unmap;
@@ -1179,6 +1181,10 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
goto err_unpin;
}
+ err = intel_engine_pool_mark_active(pool, rq);
+ if (err)
+ goto err_request;
+
err = reloc_move_to_gpu(rq, vma);
if (err)
goto err_request;
@@ -1204,7 +1210,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
cache->rq_size = 0;
/* Return with batch mapping (cmd) still pinned */
- return 0;
+ goto out_pool;
skip_request:
i915_request_skip(rq, err);
@@ -1213,7 +1219,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
err_unpin:
i915_vma_unpin(batch);
err_unmap:
- i915_gem_object_unpin_map(obj);
+ i915_gem_object_unpin_map(pool->obj);
+out_pool:
+ intel_engine_pool_put(pool);
return err;
}
@@ -1957,18 +1965,17 @@ static int i915_reset_gen7_sol_offsets(struct
i915_request *rq)
static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
{
- struct drm_i915_gem_object *shadow_batch_obj;
+ struct intel_engine_pool_node *pool;
struct i915_vma *vma;
int err;
-
