[Intel-gfx] [PATCH 11/18] drm/i915: Record space required for request emission

[Tvrtko Ursulin]

On 14/09/2016 18:33, Chris Wilson wrote:
> On Wed, Sep 14, 2016 at 02:30:20PM +0100, Tvrtko Ursulin wrote:
>> On 14/09/2016 07:52, Chris Wilson wrote:
>>> In the next patch, we will use deferred request emission. That requires
>>> reserving sufficient space in the ringbuffer to emit the request, which
>>> first requires us to know how large the request is.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem_request.c |  1 +
>>>   drivers/gpu/drm/i915/intel_lrc.c        |  6 ++++++
>>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 29 +++++++++++++++++++++++++++--
>>>   drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>>>   4 files changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
>>> index ebc2feba3e50..9a735e2d5aea 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_request.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
>>> @@ -425,6 +425,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
>>>   	 * away, e.g. because a GPU scheduler has deferred it.
>>>   	 */
>>>   	req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST;
>>> +	GEM_BUG_ON(req->reserved_space < engine->emit_request_sz);
>>>   	if (i915.enable_execlists)
>>>   		ret = intel_logical_ring_alloc_request_extras(req);
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 00fcf36ba919..7e944a0acc10 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1572,6 +1572,8 @@ static int gen8_emit_request(struct drm_i915_gem_request *request)
>>>   	return intel_logical_ring_advance(request);
>>>   }
>>> +static const int gen8_emit_request_sz = 6 + WA_TAIL_DWORDS;
>>> +
>>>   static int gen8_emit_request_render(struct drm_i915_gem_request *request)
>>>   {
>>>   	struct intel_ring *ring = request->ring;
>>> @@ -1603,6 +1605,8 @@ static int gen8_emit_request_render(struct drm_i915_gem_request *request)
>>>   	return intel_logical_ring_advance(request);
>>>   }
>>> +static const int gen8_emit_request_render_sz = 8 + WA_TAIL_DWORDS;
>>> +
>>>   static int gen8_init_rcs_context(struct drm_i915_gem_request *req)
>>>   {
>>>   	int ret;
>>> @@ -1677,6 +1681,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
>>>   	engine->reset_hw = reset_common_ring;
>>>   	engine->emit_flush = gen8_emit_flush;
>>>   	engine->emit_request = gen8_emit_request;
>>> +	engine->emit_request_sz = gen8_emit_request_sz;
>>>   	engine->submit_request = execlists_submit_request;
>>>   	engine->irq_enable = gen8_logical_ring_enable_irq;
>>> @@ -1799,6 +1804,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
>>>   	engine->init_context = gen8_init_rcs_context;
>>>   	engine->emit_flush = gen8_emit_flush_render;
>>>   	engine->emit_request = gen8_emit_request_render;
>>> +	engine->emit_request_sz = gen8_emit_request_render_sz;
>>>   	ret = intel_engine_create_scratch(engine, 4096);
>>>   	if (ret)
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> index 597e35c9b699..c8c9ad40fd93 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -1405,6 +1405,8 @@ static int i9xx_emit_request(struct drm_i915_gem_request *req)
>>>   	return 0;
>>>   }
>>> +static const int i9xx_emit_request_sz = 4;
>>> +
>>>   /**
>>>    * gen6_sema_emit_request - Update the semaphore mailbox registers
>>>    *
>>> @@ -1458,6 +1460,8 @@ static int gen8_render_emit_request(struct drm_i915_gem_request *req)
>>>   	return 0;
>>>   }
>>> +static const int gen8_render_emit_request_sz = 8;
>>> +
>>>   /**
>>>    * intel_ring_sync - sync the waiter to the signaller on seqno
>>>    *
>>> @@ -2677,8 +2681,21 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
>>>   	engine->reset_hw = reset_ring_common;
>>>   	engine->emit_request = i9xx_emit_request;
>>> -	if (i915.semaphores)
>>> +	engine->emit_request_sz = i9xx_emit_request_sz;
>>> +	if (i915.semaphores) {
>>> +		int num_rings;
>>> +
>>>   		engine->emit_request = gen6_sema_emit_request;
>>> +
>>> +		num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask) - 1;
>> You can use INTEL_INFO(dev_priv)->num_rings instead of hweight32.
> I thought info->num_rings was set afterwards? (And num_rings may be an
> overestimate here :(

You are correct, my oversight. Maybe it should have been num_rings and 
num_initialized_rings.

Regards,

Tvrtko