Commit ccd98fe4 authored by John Harrison's avatar John Harrison Committed by Daniel Vetter
Browse files

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: default avatarJohn Harrison <John.C.Harrison@Intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 4d616a29

drivers/gpu/drm/i915/i915_gem.c

+13 −12
Original line number Diff line number Diff line
@@ -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;

drivers/gpu/drm/i915/intel_lrc.c

+15 −0
Original line number Diff line number Diff line
@@ -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.

drivers/gpu/drm/i915/intel_lrc.h

+1 −0
Original line number Diff line number Diff line
@@ -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);

drivers/gpu/drm/i915/intel_ringbuffer.c

+16 −13
Original line number Diff line number Diff line
@@ -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)

drivers/gpu/drm/i915/intel_ringbuffer.h

+1 −0
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE