[Intel-gfx] [PATCH v7 1/5] drm/i915: Implement a framework for batch buffer pools
Bloomfield, Jon
jon.bloomfield at intel.com
Fri Dec 12 01:00:34 PST 2014
> -----Original Message-----
> From: Nguyen, Michael H
> Sent: Thursday, December 11, 2014 8:13 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Bloomfield, Jon; Brad Volkin
> Subject: [PATCH v7 1/5] drm/i915: Implement a framework for batch buffer
> pools
>
> From: Brad Volkin <bradley.d.volkin at intel.com>
>
> This adds a small module for managing a pool of batch buffers.
> The only current use case is for the command parser, as described in the
> kerneldoc in the patch. The code is simple, but separating it out makes it
> easier to change the underlying algorithms and to extend to future use cases
> should they arise.
>
> The interface is simple: init to create an empty pool, fini to clean it up, get to
> obtain a new buffer. Note that all buffers are expected to be inactive before
> cleaning up the pool.
>
> Locking is currently based on the caller holding the struct_mutex.
> We already do that in the places where we will use the batch pool for the
> command parser.
>
> v2:
> - s/BUG_ON/WARN_ON/ for locking assertions
> - Remove the cap on pool size
> - Switch from alloc/free to init/fini
>
> v3:
> - Idiomatic looping structure in _fini
> - Correct handling of purged objects
> - Don't return a buffer that's too much larger than needed
>
> v4:
> - Rebased to latest -nightly
>
> v5:
> - Remove _put() function and clean up comments to match
>
> v6:
> - Move purged check inside the loop (danvet, from v4 1/7 feedback)
>
> v7:
> - Use single list instead of two. (Chris W)
> - s/active_list/cache_list
> - Squashed in debug patches (Chris W)
> drm/i915: Add a batch pool debugfs file
>
> It provides some useful information about the buffers in
> the global command parser batch pool.
>
> v2: rebase on global pool instead of per-ring pools
> v3: rebase
>
> drm/i915: Add batch pool details to i915_gem_objects debugfs
>
> To better account for the potentially large memory consumption
> of the batch pool.
>
> v8:
> - Keep cache in LRU order (danvet, from v6 1/5 feedback)
>
> Issue: VIZ-4719
> Signed-off-by: Brad Volkin <bradley.d.volkin at intel.com>
> ---
> Documentation/DocBook/drm.tmpl | 5 ++
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_debugfs.c | 71 +++++++++++++--
> drivers/gpu/drm/i915/i915_drv.h | 21 +++++
> drivers/gpu/drm/i915/i915_gem.c | 1 +
> drivers/gpu/drm/i915/i915_gem_batch_pool.c | 135
> +++++++++++++++++++++++++++++
> 6 files changed, 225 insertions(+), 9 deletions(-) create mode 100644
> drivers/gpu/drm/i915/i915_gem_batch_pool.c
>
> diff --git a/Documentation/DocBook/drm.tmpl
> b/Documentation/DocBook/drm.tmpl index 25c23ca..21cbcdb 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -4059,6 +4059,11 @@ int num_ioctls;</synopsis>
> !Idrivers/gpu/drm/i915/i915_cmd_parser.c
> </sect2>
> <sect2>
> + <title>Batchbuffer Pools</title>
> +!Pdrivers/gpu/drm/i915/i915_gem_batch_pool.c batch pool
> +!Idrivers/gpu/drm/i915/i915_gem_batch_pool.c
> + </sect2>
> + <sect2>
> <title>Logical Rings, Logical Ring Contexts and Execlists</title>
> !Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and
> Execlists !Idrivers/gpu/drm/i915/intel_lrc.c
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 3cf70a6..1849ffa 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -19,6 +19,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
>
> # GEM code
> i915-y += i915_cmd_parser.o \
> + i915_gem_batch_pool.o \
> i915_gem_context.o \
> i915_gem_render_state.o \
> i915_gem_debug.o \
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 479e0c1..5d96d94 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -359,6 +359,33 @@ static int per_file_stats(int id, void *ptr, void *data)
> return 0;
> }
>
> +#define print_file_stats(m, name, stats) \
> + seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive,
> %zu global, %zu shared, %zu unbound)\n", \
> + name, \
> + stats.count, \
> + stats.total, \
> + stats.active, \
> + stats.inactive, \
> + stats.global, \
> + stats.shared, \
> + stats.unbound)
> +
> +static void print_batch_pool_stats(struct seq_file *m,
> + struct drm_i915_private *dev_priv) {
> + struct drm_i915_gem_object *obj;
> + struct file_stats stats;
> +
> + memset(&stats, 0, sizeof(stats));
> +
> + list_for_each_entry(obj,
> + &dev_priv->mm.batch_pool.cache_list,
> + batch_pool_list)
> + per_file_stats(0, obj, &stats);
> +
> + print_file_stats(m, "batch pool", stats); }
> +
> #define count_vmas(list, member) do { \
> list_for_each_entry(vma, list, member) { \
> size += i915_gem_obj_ggtt_size(vma->obj); \ @@ -441,6
> +468,9 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
>
> seq_putc(m, '\n');
> + print_batch_pool_stats(m, dev_priv);
> +
> + seq_putc(m, '\n');
> list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> struct file_stats stats;
> struct task_struct *task;
> @@ -458,15 +488,7 @@ static int i915_gem_object_info(struct seq_file *m,
> void* data)
> */
> rcu_read_lock();
> task = pid_task(file->pid, PIDTYPE_PID);
> - seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu
> inactive, %zu global, %zu shared, %zu unbound)\n",
> - task ? task->comm : "<unknown>",
> - stats.count,
> - stats.total,
> - stats.active,
> - stats.inactive,
> - stats.global,
> - stats.shared,
> - stats.unbound);
> + print_file_stats(m, task ? task->comm : "<unknown>", stats);
> rcu_read_unlock();
> }
>
> @@ -583,6 +605,36 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
> return 0;
> }
>
> +static int i915_gem_batch_pool_info(struct seq_file *m, void *data) {
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_gem_object *obj;
> + int count = 0;
> + int ret;
> +
> + ret = mutex_lock_interruptible(&dev->struct_mutex);
> + if (ret)
> + return ret;
> +
> + seq_puts(m, "cache:\n");
> + list_for_each_entry(obj,
> + &dev_priv->mm.batch_pool.cache_list,
> + batch_pool_list) {
> + seq_puts(m, " ");
> + describe_obj(m, obj);
> + seq_putc(m, '\n');
> + count++;
> + }
> +
> + seq_printf(m, "total: %d\n", count);
> +
> + mutex_unlock(&dev->struct_mutex);
> +
> + return 0;
> +}
> +
> static int i915_gem_request_info(struct seq_file *m, void *data) {
> struct drm_info_node *node = m->private; @@ -4356,6 +4408,7 @@
> static const struct drm_info_list i915_debugfs_list[] = {
> {"i915_gem_hws_blt", i915_hws_info, 0, (void *)BCS},
> {"i915_gem_hws_bsd", i915_hws_info, 0, (void *)VCS},
> {"i915_gem_hws_vebox", i915_hws_info, 0, (void *)VECS},
> + {"i915_gem_batch_pool", i915_gem_batch_pool_info, 0},
> {"i915_frequency_info", i915_frequency_info, 0},
> {"i915_drpc_info", i915_drpc_info, 0},
> {"i915_emon_status", i915_emon_status, 0}, diff --git
> a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d68c75f..e1e503a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1142,6 +1142,11 @@ struct intel_l3_parity {
> int which_slice;
> };
>
> +struct i915_gem_batch_pool {
> + struct drm_device *dev;
> + struct list_head cache_list;
> +};
> +
> struct i915_gem_mm {
> /** Memory allocator for GTT stolen memory */
> struct drm_mm stolen;
> @@ -1155,6 +1160,13 @@ struct i915_gem_mm {
> */
> struct list_head unbound_list;
>
> + /*
> + * A pool of objects to use as shadow copies of client batch buffers
> + * when the command parser is enabled. Prevents the client from
> + * modifying the batch contents after software parsing.
> + */
> + struct i915_gem_batch_pool batch_pool;
> +
> /** Usable portion of the GTT for GEM */
> unsigned long stolen_base; /* limited to low memory (32-bit) */
>
> @@ -1885,6 +1897,8 @@ struct drm_i915_gem_object {
> /** Used in execbuf to temporarily hold a ref */
> struct list_head obj_exec_link;
>
> + struct list_head batch_pool_list;
> +
> /**
> * This is set if the object is on the active lists (has pending
> * rendering and so a non-zero seqno), and is not set if it i s on @@ -
> 2889,6 +2903,13 @@ void i915_destroy_error_state(struct drm_device
> *dev); void i915_get_extra_instdone(struct drm_device *dev, uint32_t
> *instdone); const char *i915_cache_level_str(struct drm_i915_private *i915,
> int type);
>
> +/* i915_gem_batch_pool.c */
> +void i915_gem_batch_pool_init(struct drm_device *dev,
> + struct i915_gem_batch_pool *pool); void
> +i915_gem_batch_pool_fini(struct i915_gem_batch_pool *pool); struct
> +drm_i915_gem_object* i915_gem_batch_pool_get(struct
> i915_gem_batch_pool
> +*pool, size_t size);
> +
> /* i915_cmd_parser.c */
> int i915_cmd_parser_get_version(void);
> int i915_cmd_parser_init_ring(struct intel_engine_cs *ring); diff --git
> a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index de241eb..2f14ae1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4384,6 +4384,7 @@ void i915_gem_object_init(struct
> drm_i915_gem_object *obj,
> INIT_LIST_HEAD(&obj->ring_list);
> INIT_LIST_HEAD(&obj->obj_exec_link);
> INIT_LIST_HEAD(&obj->vma_list);
> + INIT_LIST_HEAD(&obj->batch_pool_list);
>
> obj->ops = ops;
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
> b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
> new file mode 100644
> index 0000000..6016125
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
> @@ -0,0 +1,135 @@
> +/*
> + * Copyright © 2014 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_drv.h"
> +
> +/**
> + * DOC: batch pool
> + *
> + * In order to submit batch buffers as 'secure', the software command
> +parser
> + * must ensure that a batch buffer cannot be modified after parsing. It
> +does
> + * this by copying the user provided batch buffer contents to a kernel
> +owned
> + * buffer from which the hardware will actually execute, and by
> +carefully
> + * managing the address space bindings for such buffers.
> + *
> + * The batch pool framework provides a mechanism for the driver to
> +manage a
> + * set of scratch buffers to use for this purpose. The framework can be
> + * extended to support other uses cases should they arise.
> + */
> +
> +/**
> + * i915_gem_batch_pool_init() - initialize a batch buffer pool
> + * @dev: the drm device
> + * @pool: the batch buffer pool
> + */
> +void i915_gem_batch_pool_init(struct drm_device *dev,
> + struct i915_gem_batch_pool *pool) {
> + pool->dev = dev;
> + INIT_LIST_HEAD(&pool->cache_list);
> +}
> +
> +/**
> + * i915_gem_batch_pool_fini() - clean up a batch buffer pool
> + * @pool: the pool to clean up
> + *
> + * Note: Callers must hold the struct_mutex.
> + */
> +void i915_gem_batch_pool_fini(struct i915_gem_batch_pool *pool) {
> + WARN_ON(!mutex_is_locked(&pool->dev->struct_mutex));
> +
> + while (!list_empty(&pool->cache_list)) {
> + struct drm_i915_gem_object *obj =
> + list_first_entry(&pool->cache_list,
> + struct drm_i915_gem_object,
> + batch_pool_list);
> +
> + WARN_ON(obj->active);
> +
> + list_del_init(&obj->batch_pool_list);
> + drm_gem_object_unreference(&obj->base);
> + }
> +}
> +
> +/**
> + * i915_gem_batch_pool_get() - select a buffer from the pool
> + * @pool: the batch buffer pool
> + * @size: the minimum desired size of the returned buffer
> + *
> + * Finds or allocates a batch buffer in the pool with at least the
> +requested
> + * size. The caller is responsible for any domain, active/inactive, or
> + * purgeability management for the returned buffer.
> + *
> + * Note: Callers must hold the struct_mutex
> + *
> + * Return: the selected batch buffer object */ struct
> +drm_i915_gem_object * i915_gem_batch_pool_get(struct
> +i915_gem_batch_pool *pool,
> + size_t size)
> +{
> + struct drm_i915_gem_object *obj = NULL;
> + struct drm_i915_gem_object *tmp, *next;
> +
> + WARN_ON(!mutex_is_locked(&pool->dev->struct_mutex));
> +
> + list_for_each_entry_safe(tmp, next,
> + &pool->cache_list, batch_pool_list) {
> +
> + if (tmp->active)
> + continue;
> +
> + /* While we're looping, do some clean up */
> + if (tmp->madv == __I915_MADV_PURGED) {
> + list_del(&tmp->batch_pool_list);
> + drm_gem_object_unreference(&tmp->base);
> + continue;
> + }
> +
> + /*
> + * Select a buffer that is at least as big as needed
> + * but not 'too much' bigger. A better way to do this
> + * might be to bucket the pool objects based on size.
> + */
> + if (tmp->base.size >= size &&
> + tmp->base.size <= (2 * size)) {
> + obj = tmp;
> + break;
> + }
> + }
> +
> + if (!obj) {
> + obj = i915_gem_alloc_object(pool->dev, size);
> + if (!obj)
> + return ERR_PTR(-ENOMEM);
> +
> + list_add_tail(&obj->batch_pool_list, &pool->cache_list);
> + }
> + else
> + /* Keep list in LRU order */
> + list_move_tail(&obj->batch_pool_list, &pool->cache_list);
> +
> + return obj;
> +}
Reviewed-By: Jon Bloomfield <jon.bloomfield at intel.com>
> --
> 1.9.1
More information about the Intel-gfx
mailing list