[PATCH] drm/i915: Remove i915_execbuffer_params

* [PATCH] drm/i915: Remove i915_execbuffer_params
@ 2017-02-03 10:33 Tvrtko Ursulin
  2017-02-03 10:46 ` Chris Wilson
  2017-02-03 12:24 ` ✗ Fi.CI.BAT: warning for " Patchwork
  0 siblings, 2 replies; 5+ messages in thread
From: Tvrtko Ursulin @ 2017-02-03 10:33 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Tidy i915_gem_do_execbuffer by removing the structure which
is under no plans to be used any longer.

This improves the redability of the code, decreases lines
of source and shrinks the binary.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 114 +++++++++++------------------
 1 file changed, 43 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a40ade6d1c16..b5f55bf858d3 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -49,17 +49,6 @@
 
 #define BATCH_OFFSET_BIAS (256*1024)
 
-struct i915_execbuffer_params {
-	struct drm_device               *dev;
-	struct drm_file                 *file;
-	struct i915_vma			*batch;
-	u32				dispatch_flags;
-	u32				args_batch_start_offset;
-	struct intel_engine_cs          *engine;
-	struct i915_gem_context         *ctx;
-	struct drm_i915_gem_request     *request;
-};
-
 struct eb_vmas {
 	struct drm_i915_private *i915;
 	struct list_head vmas;
@@ -1408,21 +1397,22 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
 }
 
 static int
-execbuf_submit(struct i915_execbuffer_params *params,
-	       struct drm_i915_gem_execbuffer2 *args,
+execbuf_submit(struct drm_i915_gem_request *req, u32 batch_start,
+	       u32 dispatch_flags, struct drm_i915_gem_execbuffer2 *args,
 	       struct list_head *vmas)
 {
-	struct drm_i915_private *dev_priv = params->request->i915;
+	struct drm_i915_private *dev_priv = req->i915;
+	struct intel_engine_cs *engine = req->engine;
 	u64 exec_start, exec_len;
 	int instp_mode;
 	u32 instp_mask;
 	int ret;
 
-	ret = i915_gem_execbuffer_move_to_gpu(params->request, vmas);
+	ret = i915_gem_execbuffer_move_to_gpu(req, vmas);
 	if (ret)
 		return ret;
 
-	ret = i915_switch_context(params->request);
+	ret = i915_switch_context(req);
 	if (ret)
 		return ret;
 
@@ -1432,25 +1422,25 @@ execbuf_submit(struct i915_execbuffer_params *params,
 	case I915_EXEC_CONSTANTS_REL_GENERAL:
 	case I915_EXEC_CONSTANTS_ABSOLUTE:
 	case I915_EXEC_CONSTANTS_REL_SURFACE:
-		if (instp_mode != 0 && params->engine->id != RCS) {
+		if (instp_mode != 0 && engine->id != RCS) {
 			DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
 			return -EINVAL;
 		}
 
 		if (instp_mode != dev_priv->relative_constants_mode) {
-			if (INTEL_INFO(dev_priv)->gen < 4) {
+			if (INTEL_GEN(dev_priv) < 4) {
 				DRM_DEBUG("no rel constants on pre-gen4\n");
 				return -EINVAL;
 			}
 
-			if (INTEL_INFO(dev_priv)->gen > 5 &&
+			if (INTEL_GEN(dev_priv) > 5 &&
 			    instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
 				DRM_DEBUG("rel surface constants mode invalid on gen5+\n");
 				return -EINVAL;
 			}
 
 			/* The HW changed the meaning on this bit on gen6 */
-			if (INTEL_INFO(dev_priv)->gen >= 6)
+			if (INTEL_GEN(dev_priv) >= 6)
 				instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
 		}
 		break;
@@ -1459,11 +1449,11 @@ execbuf_submit(struct i915_execbuffer_params *params,
 		return -EINVAL;
 	}
 
-	if (params->engine->id == RCS &&
+	if (engine->id == RCS &&
 	    instp_mode != dev_priv->relative_constants_mode) {
-		struct intel_ring *ring = params->request->ring;
+		struct intel_ring *ring = req->ring;
 
-		ret = intel_ring_begin(params->request, 4);
+		ret = intel_ring_begin(req, 4);
 		if (ret)
 			return ret;
 
@@ -1477,27 +1467,24 @@ execbuf_submit(struct i915_execbuffer_params *params,
 	}
 
 	if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
-		ret = i915_reset_gen7_sol_offsets(params->request);
+		ret = i915_reset_gen7_sol_offsets(req);
 		if (ret)
 			return ret;
 	}
 
 	exec_len   = args->batch_len;
-	exec_start = params->batch->node.start +
-		     params->args_batch_start_offset;
+	exec_start = req->batch->node.start + batch_start;
 
 	if (exec_len == 0)
-		exec_len = params->batch->size - params->args_batch_start_offset;
+		exec_len = req->batch->size - batch_start;
 
-	ret = params->engine->emit_bb_start(params->request,
-					    exec_start, exec_len,
-					    params->dispatch_flags);
+	ret = engine->emit_bb_start(req, exec_start, exec_len, dispatch_flags);
 	if (ret)
 		return ret;
 
-	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
+	trace_i915_gem_ring_dispatch(req, dispatch_flags);
 
-	i915_gem_execbuffer_move_to_active(vmas, params->request);
+	i915_gem_execbuffer_move_to_active(vmas, req);
 
 	return 0;
 }
@@ -1591,10 +1578,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	struct intel_engine_cs *engine;
 	struct i915_gem_context *ctx;
 	struct i915_address_space *vm;
-	struct i915_execbuffer_params params_master; /* XXX: will be removed later */
-	struct i915_execbuffer_params *params = &params_master;
+	struct drm_i915_gem_request *req;
+	struct i915_vma *batch;
 	const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
-	u32 dispatch_flags;
+	u32 dispatch_flags, batch_start;
 	struct dma_fence *in_fence = NULL;
 	struct sync_file *out_fence = NULL;
 	int out_fence_fd = -1;
@@ -1684,8 +1671,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	else
 		vm = &ggtt->base;
 
-	memset(&params_master, 0x00, sizeof(params_master));
-
 	eb = eb_create(dev_priv, args);
 	if (eb == NULL) {
 		i915_gem_context_put(ctx);
@@ -1700,7 +1685,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		goto err;
 
 	/* take note of the batch buffer before we might reorder the lists */
-	params->batch = eb_get_batch(eb);
+	batch = eb_get_batch(eb);
 
 	/* Move the objects en-masse into the GTT, evicting if necessary. */
 	need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
@@ -1724,24 +1709,24 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	}
 
 	/* Set the pending read domains for the batch buffer to COMMAND */
-	if (params->batch->obj->base.pending_write_domain) {
+	if (batch->obj->base.pending_write_domain) {
 		DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
 		ret = -EINVAL;
 		goto err;
 	}
-	if (args->batch_start_offset > params->batch->size ||
-	    args->batch_len > params->batch->size - args->batch_start_offset) {
+	if (args->batch_start_offset > batch->size ||
+	    args->batch_len > batch->size - args->batch_start_offset) {
 		DRM_DEBUG("Attempting to use out-of-bounds batch\n");
 		ret = -EINVAL;
 		goto err;
 	}
 
-	params->args_batch_start_offset = args->batch_start_offset;
+	batch_start = args->batch_start_offset;
 	if (engine->needs_cmd_parser && args->batch_len) {
 		struct i915_vma *vma;
 
 		vma = i915_gem_execbuffer_parse(engine, &shadow_exec_entry,
-						params->batch->obj,
+						batch->obj,
 						eb,
 						args->batch_start_offset,
 						args->batch_len,
@@ -1762,18 +1747,18 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			 * command parser has accepted.
 			 */
 			dispatch_flags |= I915_DISPATCH_SECURE;
-			params->args_batch_start_offset = 0;
-			params->batch = vma;
+			batch_start = 0;
+			batch = vma;
 		}
 	}
 
-	params->batch->obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND;
+	batch->obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND;
 
 	/* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
 	 * batch" bit. Hence we need to pin secure batches into the global gtt.
 	 * hsw should have this fixed, but bdw mucks it up again. */
 	if (dispatch_flags & I915_DISPATCH_SECURE) {
-		struct drm_i915_gem_object *obj = params->batch->obj;
+		struct drm_i915_gem_object *obj = batch->obj;
 		struct i915_vma *vma;
 
 		/*
@@ -1792,25 +1777,24 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 		}
 
-		params->batch = vma;
+		batch = vma;
 	}
 
 	/* Allocate a request for this batch buffer nice and early. */
-	params->request = i915_gem_request_alloc(engine, ctx);
-	if (IS_ERR(params->request)) {
-		ret = PTR_ERR(params->request);
+	req = i915_gem_request_alloc(engine, ctx);
+	if (IS_ERR(req)) {
+		ret = PTR_ERR(req);
 		goto err_batch_unpin;
 	}
 
 	if (in_fence) {
-		ret = i915_gem_request_await_dma_fence(params->request,
-						       in_fence);
+		ret = i915_gem_request_await_dma_fence(req, in_fence);
 		if (ret < 0)
 			goto err_request;
 	}
 
 	if (out_fence_fd != -1) {
-		out_fence = sync_file_create(&params->request->fence);
+		out_fence = sync_file_create(&req->fence);
 		if (!out_fence) {
 			ret = -ENOMEM;
 			goto err_request;
@@ -1823,27 +1807,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	 * inactive_list and lose its active reference. Hence we do not need
 	 * to explicitly hold another reference here.
 	 */
-	params->request->batch = params->batch;
+	req->batch = batch;
 
-	ret = i915_gem_request_add_to_client(params->request, file);
+	ret = i915_gem_request_add_to_client(req, file);
 	if (ret)
 		goto err_request;
 
-	/*
-	 * Save assorted stuff away to pass through to *_submission().
-	 * NB: This data should be 'persistent' and not local as it will
-	 * kept around beyond the duration of the IOCTL once the GPU
-	 * scheduler arrives.
-	 */
-	params->dev                     = dev;
-	params->file                    = file;
-	params->engine                    = engine;
-	params->dispatch_flags          = dispatch_flags;
-	params->ctx                     = ctx;
-
-	ret = execbuf_submit(params, args, &eb->vmas);
+	ret = execbuf_submit(req, batch_start, dispatch_flags, args, &eb->vmas);
 err_request:
-	__i915_add_request(params->request, ret == 0);
+	__i915_add_request(req, ret == 0);
 	if (out_fence) {
 		if (ret == 0) {
 			fd_install(out_fence_fd, out_fence->file);
@@ -1863,7 +1835,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	 * active.
 	 */
 	if (dispatch_flags & I915_DISPATCH_SECURE)
-		i915_vma_unpin(params->batch);
+		i915_vma_unpin(batch);
 err:
 	/* the request owns the ref now */
 	i915_gem_context_put(ctx);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread