Re: [PATCH 2/8] drm/i915: Make request's wait-for-space explicit

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2017-11-15 13:51:13)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > At the start of building a request, we would wait for roughly enough
>> > space to fit the average request (to reduce the likelihood of having to
>> > wait and abort partway through request construction). To achieve we
>> > would try to begin a 0-length command packet, this just adds extra
>> > confusion so make the wait-for-space explicit, as in the next patch we
>> > want to move it from the backend to the i915_gem_request_alloc() so it
>> > can ensure that the wait-for-space is the first operation in building a
>> > new request.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/intel_lrc.c        |  8 ++---
>> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 56 +++++++++++++++++++++------------
>> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>> >  3 files changed, 41 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> > index 58d050a9a866..ebd9596fe83b 100644
>> > --- a/drivers/gpu/drm/i915/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> > @@ -1180,7 +1180,7 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
>> >  {
>> >       struct intel_engine_cs *engine = request->engine;
>> >       struct intel_context *ce = &request->ctx->engine[engine->id];
>> > -     u32 *cs;
>> > +     int ret;
>> >  
>> >       GEM_BUG_ON(!ce->pin_count);
>> >  
>> > @@ -1190,9 +1190,9 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
>> >        */
>> >       request->reserved_space += EXECLISTS_REQUEST_SIZE;
>> >  
>> > -     cs = intel_ring_begin(request, 0);
>> > -     if (IS_ERR(cs))
>> > -             return PTR_ERR(cs);
>> > +     ret = intel_ring_wait_for_space(request->ring, request->reserved_space);
>> > +     if (ret)
>> > +             return ret;
>> >
>> 
>> We lose the wrap handling with this. What guarantees
>> at this point, that we dont need to wrap?
>
> We don't care. We wait for sufficient space to wrap, but we don't need
> to advance/wrap the ring pointer here as we are not emitting a packet.

Ok, and the request we alloced for, will do the needed ring_begin
and handle the wrap.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx