[Intel-gfx] [PATCH 01/43] drm/i915: Reorder the actual workload submission so that args checking is done earlier

[Chris Wilson]

On Fri, Jul 25, 2014 at 10:30:09AM +0200, Daniel Vetter wrote:
> On Thu, Jul 24, 2014 at 05:04:09PM +0100, Thomas Daniel wrote:
> > From: Oscar Mateo <oscar.mateo at intel.com>
> > 
> > In this patch:
> > 
> > commit 78382593e921c88371abd019aca8978db3248a8f
> > Author: Oscar Mateo <oscar.mateo at intel.com>
> > Date:   Thu Jul 3 16:28:05 2014 +0100
> > 
> >     drm/i915: Extract the actual workload submission mechanism from execbuffer
> > 
> >     So that we isolate the legacy ringbuffer submission mechanism, which becomes
> >     a good candidate to be abstracted away. This is prep-work for Execlists (which
> >     will its own workload submission mechanism).
> > 
> >     No functional changes.
> > 
> > I changed the order in which the args checking is done. I don't know why I did (brain
> > fade?) but itś not right. I haven't seen any ill effect from this, but the Execlists
> > version of this function will have problems if the order is not correct.
> > 
> > Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> 
> I don't think this matters - the point of no return for legacy execbuf is
> the call to ring->dispatch. After that nothing may fail any more. But as
> long as we track state correctly (e.g. if we've switched the context
> already) we'll be fine.

Right. Except that I think our tracking is buggy - or at least
insufficient to address the needs of future dispatch mechanisms. I think
that we confuse some bookkeeping that should be at the request level and
place it on the ring. At the moment, we have one request per-ring and so
it doesn't matter, but transitioning to one request per-logical-ring we
start to have issues as that state is being tracked on the wrong struct.

Anyway, that's part of the motivation to fixing up requests and making
them central to accessing the rings/dispatch (whereas at the moment they
are behind the scenes).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre