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. For: VIZ-5115 Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_gem.c | 16 ++++------------ drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++ drivers/gpu/drm/i915/intel_lrc.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 28 +++++++++++++++++----------- drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b047693..fe2de21 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2561,19 +2561,11 @@ 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. */ - ret = intel_ring_reserved_space_reserve(request->ringbuf, MIN_SPACE_FOR_ADD_REQUEST); + if (i915.enable_execlists) + ret = logical_ring_reserved_space_reserve(request); + else + ret = legacy_ring_reserved_space_reserve(request); if (ret) { /* * At this point, the request is fully allocated even if not diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index c16d726..8cb34c6 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -829,6 +829,21 @@ static int intel_logical_ring_begin(struct drm_i915_gem_request *req, return 0; } +int logical_ring_reserved_space_reserve(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..905a83e 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 logical_ring_reserved_space_reserve(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 6f198df..c7dcabd 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2205,21 +2205,27 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request) return 0; } -int intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size) +int legacy_ring_reserved_space_reserve(struct drm_i915_gem_request *request) { - /* 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);*/ + /* + * 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) +{ + 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! So just return zero and hope for the best... - */ - return 0; } void intel_ring_reserved_space_use(struct intel_ringbuffer *ringbuf, int size) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index f6ab6bb..365b98d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -442,7 +442,8 @@ intel_ring_get_request(struct intel_engine_cs *ring) #define MIN_SPACE_FOR_ADD_REQUEST 128 -int intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size); +int legacy_ring_reserved_space_reserve(struct drm_i915_gem_request *request); +void intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size); void intel_ring_reserved_space_use(struct intel_ringbuffer *ringbuf, int size); void intel_ring_reserved_space_end(struct intel_ringbuffer *ringbuf); -- 1.7.9.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx