[11/37] drm/i915/gem: Move the 'cached' info to i915_execbuffer
diff mbox series

Message ID 20200805122231.23313-12-chris@chris-wilson.co.uk
State New
Headers show
Series
  • Replace obj->mm.lock with reservation_ww_class
Related show

Commit Message

Chris Wilson Aug. 5, 2020, 12:22 p.m. UTC
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@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(-)

Comments

Tvrtko Ursulin Aug. 5, 2020, 1:29 p.m. UTC | #1
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@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@intel.com>

Regards,

Tvrtko

Patch
diff mbox series

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);