[Intel-gfx] [PATCH 11/37] drm/i915/gem: Move the 'cached' info to i915_execbuffer
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Wed Aug 5 13:29:38 UTC 2020
On 05/08/2020 13:22, Chris Wilson wrote:
> The reloc_cache contains some details that are used outside of the
> relocation handling, so lift those out of the embeddded struct into the
> principle struct i915_execbuffer.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 61 +++++++++++--------
> .../i915/gem/selftests/i915_gem_execbuffer.c | 6 +-
> 2 files changed, 37 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index e7e16c62df1c..e9ef0c287fd9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -261,11 +261,6 @@ struct i915_execbuffer {
> */
> struct reloc_cache {
> struct drm_mm_node node; /** temporary GTT binding */
> - unsigned int gen; /** Cached value of INTEL_GEN */
> - bool use_64bit_reloc : 1;
> - bool has_llc : 1;
> - bool has_fence : 1;
> - bool needs_unfenced : 1;
>
> struct intel_context *ce;
>
> @@ -283,6 +278,12 @@ struct i915_execbuffer {
> u32 batch_len; /** Length of batch within object */
> u32 batch_flags; /** Flags composed for emit_bb_start() */
>
> + unsigned int gen; /** Cached value of INTEL_GEN */
> + bool use_64bit_reloc : 1;
> + bool has_llc : 1;
> + bool has_fence : 1;
> + bool needs_unfenced : 1;
> +
> /**
> * Indicate either the size of the hastable used to resolve
> * relocation handles, or if negative that we are using a direct
> @@ -540,11 +541,11 @@ eb_validate_vma(struct i915_execbuffer *eb,
> */
> entry->offset = gen8_noncanonical_addr(entry->offset);
>
> - if (!eb->reloc_cache.has_fence) {
> + if (!eb->has_fence) {
> entry->flags &= ~EXEC_OBJECT_NEEDS_FENCE;
> } else {
> if ((entry->flags & EXEC_OBJECT_NEEDS_FENCE ||
> - eb->reloc_cache.needs_unfenced) &&
> + eb->needs_unfenced) &&
> i915_gem_object_is_tiled(vma->obj))
> entry->flags |= EXEC_OBJECT_NEEDS_GTT | __EXEC_OBJECT_NEEDS_MAP;
> }
> @@ -592,7 +593,7 @@ eb_add_vma(struct i915_execbuffer *eb,
> if (entry->relocation_count &&
> !(ev->flags & EXEC_OBJECT_PINNED))
> ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
> - if (eb->reloc_cache.has_fence)
> + if (eb->has_fence)
> ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
>
> eb->batch = ev;
> @@ -995,15 +996,19 @@ relocation_target(const struct drm_i915_gem_relocation_entry *reloc,
> return gen8_canonical_addr((int)reloc->delta + target->node.start);
> }
>
> -static void reloc_cache_init(struct reloc_cache *cache,
> - struct drm_i915_private *i915)
> +static void eb_info_init(struct i915_execbuffer *eb,
> + struct drm_i915_private *i915)
> {
> /* Must be a variable in the struct to allow GCC to unroll. */
> - cache->gen = INTEL_GEN(i915);
> - cache->has_llc = HAS_LLC(i915);
> - cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
> - cache->has_fence = cache->gen < 4;
> - cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> + eb->gen = INTEL_GEN(i915);
> + eb->has_llc = HAS_LLC(i915);
> + eb->use_64bit_reloc = HAS_64BIT_RELOC(i915);
> + eb->has_fence = eb->gen < 4;
> + eb->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> +}
> +
> +static void reloc_cache_init(struct reloc_cache *cache)
> +{
> cache->node.flags = 0;
> cache->rq = NULL;
> cache->target = NULL;
> @@ -1011,8 +1016,9 @@ static void reloc_cache_init(struct reloc_cache *cache,
>
> #define RELOC_TAIL 4
>
> -static int reloc_gpu_chain(struct reloc_cache *cache)
> +static int reloc_gpu_chain(struct i915_execbuffer *eb)
> {
> + struct reloc_cache *cache = &eb->reloc_cache;
> struct intel_gt_buffer_pool_node *pool;
> struct i915_request *rq = cache->rq;
> struct i915_vma *batch;
> @@ -1036,9 +1042,9 @@ static int reloc_gpu_chain(struct reloc_cache *cache)
> GEM_BUG_ON(cache->rq_size + RELOC_TAIL > PAGE_SIZE / sizeof(u32));
> cmd = cache->rq_cmd + cache->rq_size;
> *cmd++ = MI_ARB_CHECK;
> - if (cache->gen >= 8)
> + if (eb->gen >= 8)
> *cmd++ = MI_BATCH_BUFFER_START_GEN8;
> - else if (cache->gen >= 6)
> + else if (eb->gen >= 6)
> *cmd++ = MI_BATCH_BUFFER_START;
> else
> *cmd++ = MI_BATCH_BUFFER_START | MI_BATCH_GTT;
> @@ -1061,7 +1067,7 @@ static int reloc_gpu_chain(struct reloc_cache *cache)
> goto out_pool;
>
> cmd = i915_gem_object_pin_map(batch->obj,
> - cache->has_llc ?
> + eb->has_llc ?
> I915_MAP_FORCE_WB :
> I915_MAP_FORCE_WC);
> if (IS_ERR(cmd)) {
> @@ -1110,9 +1116,9 @@ static void __i915_request_add(struct i915_request *rq,
> __i915_request_queue(rq, attr);
> }
>
> -static unsigned int reloc_bb_flags(const struct reloc_cache *cache)
> +static unsigned int reloc_bb_flags(const struct i915_execbuffer *eb)
> {
> - return cache->gen > 5 ? 0 : I915_DISPATCH_SECURE;
> + return eb->gen > 5 ? 0 : I915_DISPATCH_SECURE;
> }
>
> static int reloc_gpu_flush(struct i915_execbuffer *eb)
> @@ -1143,7 +1149,7 @@ static int reloc_gpu_flush(struct i915_execbuffer *eb)
> err = rq->engine->emit_bb_start(rq,
> rq->batch->node.start,
> PAGE_SIZE,
> - reloc_bb_flags(cache));
> + reloc_bb_flags(eb));
> if (err)
> i915_request_set_error_once(rq, err);
>
> @@ -1191,7 +1197,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
> return PTR_ERR(pool);
>
> cmd = i915_gem_object_pin_map(pool->obj,
> - cache->has_llc ?
> + eb->has_llc ?
> I915_MAP_FORCE_WB :
> I915_MAP_FORCE_WC);
> if (IS_ERR(cmd)) {
> @@ -1285,7 +1291,7 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
>
> if (unlikely(cache->rq_size + len >
> PAGE_SIZE / sizeof(u32) - RELOC_TAIL)) {
> - err = reloc_gpu_chain(cache);
> + err = reloc_gpu_chain(eb);
> if (unlikely(err)) {
> i915_request_set_error_once(cache->rq, err);
> return ERR_PTR(err);
> @@ -1318,7 +1324,7 @@ static int __reloc_entry_gpu(struct i915_execbuffer *eb,
> u64 offset,
> u64 target_addr)
> {
> - const unsigned int gen = eb->reloc_cache.gen;
> + const unsigned int gen = eb->gen;
> unsigned int len;
> u32 *batch;
> u64 addr;
> @@ -1465,7 +1471,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>
> /* Check that the relocation address is valid... */
> if (unlikely(reloc->offset >
> - ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
> + ev->vma->size - (eb->use_64bit_reloc ? 8 : 4))) {
> drm_dbg(&i915->drm, "Relocation beyond object bounds: "
> "target %d offset %d size %d.\n",
> reloc->target_handle,
> @@ -2676,7 +2682,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> eb.exec = exec;
>
> eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
> - reloc_cache_init(&eb.reloc_cache, eb.i915);
> + eb_info_init(&eb, eb.i915);
> + reloc_cache_init(&eb.reloc_cache);
>
> eb.buffer_count = args->buffer_count;
> eb.batch_start_offset = args->batch_start_offset;
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> index d87f572048e8..13f17d6b5c66 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> @@ -23,8 +23,7 @@ static int __igt_gpu_reloc(struct i915_execbuffer *eb,
> struct drm_i915_gem_object *obj)
> {
> const unsigned int offsets[] = { 8, 3, 0 };
> - const u64 mask =
> - GENMASK_ULL(eb->reloc_cache.use_64bit_reloc ? 63 : 31, 0);
> + const u64 mask = GENMASK_ULL(eb->use_64bit_reloc ? 63 : 31, 0);
> const u32 *map = page_mask_bits(obj->mm.mapping);
> struct i915_request *rq;
> struct i915_vma *vma;
> @@ -126,8 +125,9 @@ static int igt_gpu_reloc(void *arg)
> goto err_scratch;
> }
>
> + eb_info_init(&eb, eb.i915);
> for_each_uabi_engine(eb.engine, eb.i915) {
> - reloc_cache_init(&eb.reloc_cache, eb.i915);
> + reloc_cache_init(&eb.reloc_cache);
> memset(map, POISON_INUSE, 4096);
>
> intel_engine_pm_get(eb.engine);
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list