[Intel-gfx] [PATCH 48/55] drm/i915: Add *_ring_begin() to request allocation

[Daniel Vetter]

On Wed, Jun 17, 2015 at 03:27:08PM +0100, Chris Wilson wrote:
> On Wed, Jun 17, 2015 at 03:31:59PM +0200, Daniel Vetter wrote:
> > On Fri, May 29, 2015 at 05:44:09PM +0100, John.C.Harrison at Intel.com wrote:
> > > From: John Harrison <John.C.Harrison at Intel.com>
> > > 
> > > 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 at Intel.com>
> > 
> > Still has my question open from the previos round:
> > 
> > http://mid.gmane.org/20150323091030.GL1349@phenom.ffwll.local
> > 
> > Note that this isn't all that unlikely with GuC mode since there the
> > ringbuffer is substantially smaller (due to firmware limitations) than
> > what we allocate ourselves right now.
> 
> Looking at this patch, I am still fundamentally opposed to reserving
> space for the request. Detecting a request that wraps and cancelling
> that request (after the appropriate WARN for the overlow) is trivial and
> such a rare case (as it is a programming error) that it should only be
> handled in the slow path.

I thought the entire point here that we don't have request half-committed
because the final request ringcmds didn't fit in. And that does require
that we reserve a bit of space for that postamble.

I guess if it's too much (atm it's super-pessimistic due to ilk) we can
make per-platform reservation limits to be really minimal.

Maybe we could go towards a rollback model longterm of rewingind the
ringbuffer. But if there's no clear need I'd like to avoid that
complexity.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch