Re: [PATCH 3/6] drm/i915: simplify allocation of driver-internal requests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22/12/15 09:08, Chris Wilson wrote:
On Mon, Dec 21, 2015 at 04:04:42PM +0000, Dave Gordon wrote:
There are a number of places where the driver needs a request, but isn't
working on behalf of any specific user or in a specific context. For
such requests, we associate them with the default context for the engine
that the request will be submitted to.

This patch provides a shorthand for doing such request allocations and
changes all such calls to use the new function.

Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
  drivers/gpu/drm/i915/i915_gem.c      | 36 ++++++++++++++++++++++++++++--------
  drivers/gpu/drm/i915/intel_display.c |  6 ++++--
  drivers/gpu/drm/i915/intel_overlay.c | 24 ++++++++++++------------
  4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 666d07c..4955db9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2270,6 +2270,8 @@ struct drm_i915_gem_request {
  int i915_gem_request_alloc(struct intel_engine_cs *ring,
  			   struct intel_context *ctx,
  			   struct drm_i915_gem_request **req_out);
+struct drm_i915_gem_request * __must_check
+	i915_gem_request_alloc_anon(struct intel_engine_cs *ring);
  void i915_gem_request_cancel(struct drm_i915_gem_request *req);
  void i915_gem_request_free(struct kref *req_ref);
  int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index be1f984..9f9c0c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2751,6 +2751,21 @@ err:
  	return ret;
  }

+/*
+ * Allocate a request associated with the default context for the given
+ * ring. This can be used where the driver needs a request for internal
+ * purposes not directly related to a user batch submission.
+ */
+struct drm_i915_gem_request *
+i915_gem_request_alloc_anon(struct intel_engine_cs *ring)
+{

As demonstrated, no. Contexts need to be considered properly first.

+	struct drm_i915_gem_request *req;
+	int err;
+
+	err = i915_gem_request_alloc(ring, ring->default_context, &req);
+	return err ? ERR_PTR(err) : req;
+}
+
  void i915_gem_request_cancel(struct drm_i915_gem_request *req)
  {
  	intel_ring_reserved_space_cancel(req->ringbuf);
@@ -3168,9 +3183,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj,
  			return 0;

  		if (*to_req == NULL) {
-			ret = i915_gem_request_alloc(to, to->default_context, to_req);
-			if (ret)
-				return ret;
+			struct drm_i915_gem_request *req;
+
+			req = i915_gem_request_alloc_anon(to);

Wrong context. Please see patches to fix this mess.

+			if (IS_ERR(req))
+				return PTR_ERR(req);
+
+			*to_req = req;
  		}

  		trace_i915_gem_ring_sync_to(*to_req, from, from_req);
@@ -3370,9 +3389,9 @@ int i915_gpu_idle(struct drm_device *dev)
  		if (!i915.enable_execlists) {
  			struct drm_i915_gem_request *req;

-			ret = i915_gem_request_alloc(ring, ring->default_context, &req);
-			if (ret)
-				return ret;
+			req = i915_gem_request_alloc_anon(ring);
+			if (IS_ERR(req))
+				return PTR_ERR(req);

  			ret = i915_switch_context(req);
  			if (ret) {
@@ -4867,8 +4886,9 @@ i915_gem_init_hw(struct drm_device *dev)
  	for_each_ring(ring, dev_priv, i) {
  		struct drm_i915_gem_request *req;

-		ret = i915_gem_request_alloc(ring, ring->default_context, &req);
-		if (ret) {
+		req = i915_gem_request_alloc_anon(ring);
+		if (IS_ERR(req)) {
+			ret = PTR_ERR(req);
  			i915_gem_cleanup_ringbuffer(dev);
  			goto out;
  		}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index abd2d29..5716f4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
  					obj->last_write_req);
  	} else {
  		if (!request) {
-			ret = i915_gem_request_alloc(ring, ring->default_context, &request);
-			if (ret)

Wrong context.
-Chris

What do you mean, "wrong context". The line above is the way it *currently* works, which I'm replacing with one that *doesn't* refer to the default context.

This patch doesn't change any functionality, just simplifies it by reducing the number of instances of "default_context" so that patch 4/6 can actually get rid of ring->default_context entirely. I thought that was what you were aiming for?

.Dave.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux