drm/i915: Add *_ring_begin() to request allocation

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@Intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e7a40f8712bc5a7bcceee21b2f4e825af48a5e0f..0af4960d141c23c9ccb660f99686ab85b453d624 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2679,19 +2679,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 ad9db8a79d080620f3cde94dbbd5e643573d05be..75c3b9dfec9eb306a633bb4e069c9d28ddf53bc0 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -834,6 +834,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 044c0e5c72e5f16529c97bdfbf2ff92d50699466..f59940ac1cfc7183cd5bc7815df789b10892d967 100644 (file)
--- 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 dfba3ee5738278c23e5cdae85a15fc2559f975a9..a378360d0461ec45defd72f3a6e2fe97e4a5cbb6 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2202,24 +2202,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 00a4ff7593ce74ece29f896e3b3cb42f9aa578cc..0f12020b54e9e6051897a4d0bfedba00c0a0b752 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -484,6 +484,7 @@ intel_ring_get_request(struct intel_engine_cs *ring)
  * will always have sufficient room to do its stuff. The request creation
  * code calls this automatically.
  */
+int intel_ring_reserve_space(struct drm_i915_gem_request *request);
 void intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size);
 /* Cancel the reservation, e.g. because the request is being discarded. */
 void intel_ring_reserved_space_cancel(struct intel_ringbuffer *ringbuf);