public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tursulin@ursulin.net>
To: Intel-gfx@lists.freedesktop.org
Subject: [PATCH 01/10] drm/i915: Tidy i915_gem_do_execbuffer
Date: Tue, 31 Jan 2017 13:15:37 +0000	[thread overview]
Message-ID: <1485868546-4927-2-git-send-email-tvrtko.ursulin@linux.intel.com> (raw)
In-Reply-To: <1485868546-4927-1-git-send-email-tvrtko.ursulin@linux.intel.com>

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

Instead of sprinkling around usage and initialization of
i915_execbuffer_params we can consolidate it just before
execbuf_submit for maintability and readability.

That way we can also drop the memset since it becomes
easy to spot we initialize all the fields.

     text    data     bss     dec     hex filename
  1085466   26398    2628 1114492  11017c i915.ko.0
  1085402   26398    2628 1114428  11013c i915.ko.1

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 87 +++++++++++++++---------------
 1 file changed, 44 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 91c2393199a3..ee7b7bd17e29 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -50,14 +50,14 @@
 #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 drm_device	    *dev;
+	struct drm_file		    *file;
+	struct i915_vma		    *batch;
+	u32			    dispatch_flags;
+	u32		    	    batch_start;
+	struct intel_engine_cs	    *engine;
+	struct i915_gem_context	    *ctx;
+	struct drm_i915_gem_request *request;
 };
 
 struct eb_vmas {
@@ -1484,11 +1484,10 @@ execbuf_submit(struct i915_execbuffer_params *params,
 	}
 
 	exec_len   = args->batch_len;
-	exec_start = params->batch->node.start +
-		     params->args_batch_start_offset;
+	exec_start = params->batch->node.start + params->batch_start;
 
 	if (exec_len == 0)
-		exec_len = params->batch->size - params->args_batch_start_offset;
+		exec_len = params->batch->size - params->batch_start;
 
 	ret = params->engine->emit_bb_start(params->request,
 					    exec_start, exec_len,
@@ -1592,8 +1591,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;
+	u32 batch_start_offset;
+	struct i915_execbuffer_params params;
 	const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
 	u32 dispatch_flags;
 	struct dma_fence *in_fence = NULL;
@@ -1685,8 +1686,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);
@@ -1701,7 +1700,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;
@@ -1725,24 +1724,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_offset = 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,
@@ -1763,18 +1762,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_offset = 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;
 
 		/*
@@ -1793,25 +1792,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;
@@ -1824,9 +1822,9 @@ 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;
 
@@ -1836,15 +1834,18 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	 * 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);
+	params.dev	      = dev;
+	params.file	      = file;
+	params.engine	      = engine;
+	params.dispatch_flags = dispatch_flags;
+	params.ctx	      = ctx;
+	params.batch	      = batch;
+	params.batch_start    = batch_start_offset;
+	params.request	      = req;
+
+	ret = execbuf_submit(&params, 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);
@@ -1864,7 +1865,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

  reply	other threads:[~2017-01-31 13:15 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-31 13:15 [PATCH 00/10] do_execbuffer tidy Tvrtko Ursulin
2017-01-31 13:15 ` Tvrtko Ursulin [this message]
2017-02-01  7:14   ` [PATCH 01/10] drm/i915: Tidy i915_gem_do_execbuffer Joonas Lahtinen
2017-01-31 13:15 ` [PATCH 02/10] drm/i915: Drop some unused fields from i915_execbuffer_params Tvrtko Ursulin
2017-02-01  7:20   ` Joonas Lahtinen
2017-02-01 16:08     ` Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 03/10] drm/i915: Tidy execbuf_submit Tvrtko Ursulin
2017-02-01  7:19   ` Joonas Lahtinen
2017-01-31 13:15 ` [PATCH 04/10] drm/i915: Remove batch field from i915_execbuffer_params Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 05/10] drm/i915: Nuke i915_execbuffer_params Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 06/10] drm/i915: Remove some single use locals i915_gem_do_execbuffer Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 07/10] drm/i915: Drop unused engine parameter from i915_gem_validate_context Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 08/10] drm/i915: eb_engine_select only needs flags Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 09/10] drm/i915: Pass file_priv to eb_select_engine Tvrtko Ursulin
2017-01-31 13:15 ` [PATCH 10/10] drm/i915: Remove some unecessary line breaks Tvrtko Ursulin
2017-01-31 13:20 ` [PATCH 00/10] do_execbuffer tidy Chris Wilson
2017-01-31 15:24 ` ✗ Fi.CI.BAT: warning for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1485868546-4927-2-git-send-email-tvrtko.ursulin@linux.intel.com \
    --to=tursulin@ursulin.net \
    --cc=Intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox