[Intel-gfx] [PATCH 50/70] drm/i915: The argument for postfix is redundant

[Chris Wilson]

On Fri, Apr 10, 2015 at 11:32:36AM +0200, Daniel Vetter wrote:
> On Fri, Apr 10, 2015 at 10:00:33AM +0100, Chris Wilson wrote:
> > On Fri, Apr 10, 2015 at 10:53:02AM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 07, 2015 at 04:21:14PM +0100, Chris Wilson wrote:
> > > > We are conservative on the amount of free space available in the ring to
> > > > avoid overruning the potential MI_INTERRUPT after the seqno write.
> > > > Further undermining the justification for the change was that it was
> > > > applied incorrectly.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > 
> > > Hm, where are we conservative with our estimates? Could we just wait for
> > > req->head instead? And I don't see what's been implemented wrongly with
> > > postfix.
> > 
> > It took more patches for it to get fixed, when it wasn't actually broken as
> > the calculation of space remaining is conservative. req->head would be a
> > reasonable compromise to the addition of another variable, and the extra bit
> > of hysteresis here would probably be useful. Hmm.
> > 
> > > Looking at
> > > 
> > > commit a71d8d94525e8fd855c0466fb586ae1cb008f3a2
> > > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > > Date:   Wed Feb 15 11:25:36 2012 +0000
> > > 
> > >     drm/i915: Record the tail at each request and use it to estimate the head
> > > 
> > > ->postfix does get updated where ->tail was, Nick just renamed it from
> > > ->tail to ->postfix since execlist used ->tail with a different meaning.
> > > 
> > > Or do I miss something?
> > 
> > tail was always the position of the end of the request.
> 
> In the above referenced commit request->tail is sampled _before_ we call
> ->add_request and all that stuff. That seems to have changed in
> 
> commit 6d3d8274bc45de4babb62d64562d92af984dd238
> Author: Nick Hoath <nicholas.hoath at intel.com>
> Date:   Thu Jan 15 13:10:39 2015 +0000
> 
>     drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
> 
> by essentially doing an s/request->tail/request->postfix/ and adding a new
> request->tail (real tail) to satisfy execlist.

Oh sorry, execlists was broken. Quel surprise. The model should have
been to add the breadcrumb, grab the tail for the end of the request,
submit to engine

http://cgit.freedesktop.org/~ickle/linux-2.6/tree/drivers/gpu/drm/i915/i915_gem_request.c#n378
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre