[Intel-gfx] [PATCH 1/2] drm/i915: Split adding request to smaller functions

[Mika Kuoppala]

Clean __i915_add_request by splitting request submission to
preparation, actual submission and adding to client.

While doing this we can remove the request->start which
was not used.

Cc: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 116 +++++++++++++++++++++++++++-------------
 1 file changed, 78 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 61134ab..06265e7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2407,26 +2407,34 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 	return 0;
 }
 
-int __i915_add_request(struct intel_engine_cs *ring,
-		       struct drm_file *file,
-		       struct drm_i915_gem_object *obj)
+static struct intel_ringbuffer *
+__request_to_ringbuf(struct drm_i915_gem_request *request)
+{
+	if (i915.enable_execlists)
+		return request->ctx->engine[request->ring->id].ringbuf;
+
+	return request->ring->buffer;
+}
+
+static struct drm_i915_gem_request *
+i915_gem_request_prepare(struct intel_engine_cs *ring, struct drm_file *file)
 {
-	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct drm_i915_gem_request *request;
 	struct intel_ringbuffer *ringbuf;
-	u32 request_start;
 	int ret;
 
 	request = ring->outstanding_lazy_request;
 	if (WARN_ON(request == NULL))
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	if (i915.enable_execlists) {
-		ringbuf = request->ctx->engine[ring->id].ringbuf;
-	} else
-		ringbuf = ring->buffer;
+	/* execlist submission has this already set */
+	if (!request->ctx)
+		request->ctx = ring->last_context;
+
+	ringbuf = __request_to_ringbuf(request);
+	if (WARN_ON(ringbuf == NULL))
+		return ERR_PTR(-EIO);
 
-	request_start = intel_ring_get_tail(ringbuf);
 	/*
 	 * Emit any outstanding flushes - execbuf can fail to emit the flush
 	 * after having emitted the batchbuffer command. Hence we need to fix
@@ -2434,21 +2442,30 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	 * is that the flush _must_ happen before the next request, no matter
 	 * what.
 	 */
-	if (i915.enable_execlists) {
+	if (i915.enable_execlists)
 		ret = logical_ring_flush_all_caches(ringbuf, request->ctx);
-		if (ret)
-			return ret;
-	} else {
+	else
 		ret = intel_ring_flush_all_caches(ring);
-		if (ret)
-			return ret;
-	}
+
+	if (ret)
+		return ERR_PTR(ret);
+
+	return request;
+}
+
+static int i915_gem_request_submit(struct drm_i915_gem_request *request,
+				   struct drm_i915_gem_object *batch)
+{
+	struct intel_ringbuffer *ringbuf = __request_to_ringbuf(request);
+	struct intel_engine_cs *ring = request->ring;
+	int ret;
 
 	/* Record the position of the start of the request so that
 	 * should we detect the updated seqno part-way through the
 	 * GPU processing the request, we never over-estimate the
 	 * position of the head.
 	 */
+	request->batch_obj = batch;
 	request->postfix = intel_ring_get_tail(ringbuf);
 
 	if (i915.enable_execlists) {
@@ -2461,7 +2478,6 @@ int __i915_add_request(struct intel_engine_cs *ring,
 			return ret;
 	}
 
-	request->head = request_start;
 	request->tail = intel_ring_get_tail(ringbuf);
 
 	/* Whilst this request exists, batch_obj will be on the
@@ -2470,35 +2486,59 @@ int __i915_add_request(struct intel_engine_cs *ring,
 	 * inactive_list and lose its active reference. Hence we do not need
 	 * to explicitly hold another reference here.
 	 */
-	request->batch_obj = obj;
 
-	if (!i915.enable_execlists) {
-		/* Hold a reference to the current context so that we can inspect
-		 * it later in case a hangcheck error event fires.
+	if (!i915.enable_execlists && request->ctx) {
+		/* Hold a reference to the current context so that we can
+		 * inspect it later in case a hangcheck error event fires.
 		 */
-		request->ctx = ring->last_context;
-		if (request->ctx)
-			i915_gem_context_reference(request->ctx);
+		i915_gem_context_reference(request->ctx);
 	}
 
 	request->emitted_jiffies = jiffies;
+
 	list_add_tail(&request->list, &ring->request_list);
-	request->file_priv = NULL;
+	ring->outstanding_lazy_request = NULL;
 
-	if (file) {
-		struct drm_i915_file_private *file_priv = file->driver_priv;
+	trace_i915_gem_request_add(request);
 
-		spin_lock(&file_priv->mm.lock);
-		request->file_priv = file_priv;
-		list_add_tail(&request->client_list,
-			      &file_priv->mm.request_list);
-		spin_unlock(&file_priv->mm.lock);
+	return 0;
+}
 
-		request->pid = get_pid(task_pid(current));
-	}
+static void i915_gem_request_add_to_client(struct drm_i915_gem_request *request)
+{
+	struct drm_i915_file_private *file_priv;
 
-	trace_i915_gem_request_add(request);
-	ring->outstanding_lazy_request = NULL;
+	if (!request->file_priv)
+		return;
+
+	file_priv = request->file_priv;
+
+	spin_lock(&file_priv->mm.lock);
+	request->file_priv = file_priv;
+	list_add_tail(&request->client_list,
+		      &file_priv->mm.request_list);
+	spin_unlock(&file_priv->mm.lock);
+
+	request->pid = get_pid(task_pid(current));
+}
+
+int __i915_add_request(struct intel_engine_cs *ring,
+		       struct drm_file *file,
+		       struct drm_i915_gem_object *batch)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+	struct drm_i915_gem_request *request;
+	int ret;
+
+	request = i915_gem_request_prepare(ring, file);
+	if (IS_ERR(request))
+		return PTR_ERR(request);
+
+	ret = i915_gem_request_submit(request, batch);
+	if (ret)
+		return ret;
+
+	i915_gem_request_add_to_client(request);
 
 	i915_queue_hangcheck(ring->dev);
 
-- 
1.9.1