From: John Harrison <John.C.Harrison@xxxxxxxxx>
Now that the *_ring_begin() functions no longer call the request allocation
code, it is finally safe for the request allocation code to call *_ring_begin().
This is important to guarantee that the space reserved for the subsequent
i915_add_request() call does actually get reserved.
v2: Renamed functions according to review feedback (Tomas Elf).
For: VIZ-5115
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 25 +++++++++++++------------
drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
drivers/gpu/drm/i915/intel_lrc.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 29 ++++++++++++++++-------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
5 files changed, 46 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9f3e0717..1261792 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2680,19 +2680,20 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring,
* i915_add_request() call can't fail. Note that the reserve may need
* to be redone if the request is not actually submitted straight
* away, e.g. because a GPU scheduler has deferred it.
- *
- * Note further that this call merely notes the reserve request. A
- * subsequent call to *_ring_begin() is required to actually ensure
- * that the reservation is available. Without the begin, if the
- * request creator immediately submitted the request without adding
- * any commands to it then there might not actually be sufficient
- * room for the submission commands. Unfortunately, the current
- * *_ring_begin() implementations potentially call back here to
- * i915_gem_request_alloc(). Thus calling _begin() here would lead to
- * infinite recursion! Until that back call path is removed, it is
- * necessary to do a manual _begin() outside.
*/
- intel_ring_reserved_space_reserve(req->ringbuf, MIN_SPACE_FOR_ADD_REQUEST);
+ if (i915.enable_execlists)
+ ret = intel_logical_ring_reserve_space(req);
+ else
+ ret = intel_ring_reserve_space(req);
+ if (ret) {
+ /*
+ * At this point, the request is fully allocated even if not
+ * fully prepared. Thus it can be cleaned up using the proper
+ * free code.
+ */
+ i915_gem_request_cancel(req);
+ return ret;
+ }
*req_out = ring->outstanding_lazy_request = req;
return 0;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 548c53d..e164ac0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -823,6 +823,21 @@ static int intel_logical_ring_begin(struct drm_i915_gem_request *req,
return 0;
}
+int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request)
+{
+ /*
+ * The first call merely notes the reserve request and is common for
+ * all back ends. The subsequent localised _begin() call actually
+ * ensures that the reservation is available. Without the begin, if
+ * the request creator immediately submitted the request without
+ * adding any commands to it then there might not actually be
+ * sufficient room for the submission commands.
+ */
+ intel_ring_reserved_space_reserve(request->ringbuf, MIN_SPACE_FOR_ADD_REQUEST);
+
+ return intel_logical_ring_begin(request, 0);
+}
+
/**
* execlists_submission() - submit a batchbuffer for execution, Execlists style
* @dev: DRM device.
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 044c0e5..f59940a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -37,6 +37,7 @@
/* Logical Rings */
int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
+int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
void intel_logical_ring_stop(struct intel_engine_cs *ring);
void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
int intel_logical_rings_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index bb10fc2..0ba5787 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2192,24 +2192,27 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
return 0;
}
+int intel_ring_reserve_space(struct drm_i915_gem_request *request)
+{
+ /*
+ * The first call merely notes the reserve request and is common for
+ * all back ends. The subsequent localised _begin() call actually
+ * ensures that the reservation is available. Without the begin, if
+ * the request creator immediately submitted the request without
+ * adding any commands to it then there might not actually be
+ * sufficient room for the submission commands.
+ */
+ intel_ring_reserved_space_reserve(request->ringbuf, MIN_SPACE_FOR_ADD_REQUEST);
+
+ return intel_ring_begin(request, 0);
+}
+
void intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size)
{
- /* NB: Until request management is fully tidied up and the OLR is
- * removed, there are too many ways for get false hits on this
- * anti-recursion check! */
- /*WARN_ON(ringbuf->reserved_size);*/
+ WARN_ON(ringbuf->reserved_size);
WARN_ON(ringbuf->reserved_in_use);
ringbuf->reserved_size = size;
-
- /*
- * Really need to call _begin() here but that currently leads to
- * recursion problems! This will be fixed later but for now just
- * return and hope for the best. Note that there is only a real
- * problem if the create of the request never actually calls _begin()
- * but if they are not submitting any work then why did they create
- * the request in the first place?
- */
}
void intel_ring_reserved_space_cancel(struct intel_ringbuffer *ringbuf)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 16fd9ba..f4633ca 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -450,6 +450,7 @@ intel_ring_get_request(struct intel_engine_cs *ring)
#define MIN_SPACE_FOR_ADD_REQUEST 128
+int intel_ring_reserve_space(struct drm_i915_gem_request *request);
void intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size);
void intel_ring_reserved_space_cancel(struct intel_ringbuffer *ringbuf);
void intel_ring_reserved_space_use(struct intel_ringbuffer *ringbuf);
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
