[Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload submission mechanism from execbuffer

[Chris Wilson]

On Thu, Jul 03, 2014 at 09:07:41AM +0000, Mateo Lozano, Oscar wrote:
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > Sent: Thursday, July 03, 2014 8:32 AM
> > To: Mateo Lozano, Oscar
> > Cc: intel-gfx at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload
> > submission mechanism from execbuffer
> > 
> > On Thu, Jun 26, 2014 at 02:24:19PM +0100, oscar.mateo at intel.com wrote:
> > > +	i915_gem_execbuffer_move_to_active(vmas, ring);
> > > +	i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
> > 
> > This is where I start freaking out over the mix of global vs logical state and
> > the implications of reordering.
> 
> I hear you, Chris. This is scary stuff because it has a lot of implications, but it has been in review for a long time and we seem to be stalled.
> 
> > The key question for me is how clean busy-ioctl is when it has to pick up the
> > pieces from a partial failure to submit.
> 
> AFAICT, __i915_add_request() needs some changes if we don´t want to reuse the intel_ringbuffer.c functionality (Daniel was wondering why I needed to abstract __i915_add_request away with another vfunc: this is the reason) and i915_gem_retire_requests_ring() needs to update last_retired_head in the correct ringbuf (global or logical). Other than that, everything seems healthy, as things like the list of active objects and the list of requests reside on intel_engine_cs, so they are common for both paths.
> 
> How could I put your mind at ease? maybe by creating a topic branch and going through QA for a while? or merging it as disabled by default? I don´t feel rewriting it forever is going to make it any better :(

At the moment, I feel happy to merge as-is and then folding back in the new
functionality. The problem with relying solely on i-g-t here is that I
am concerned about the nasty sort of races that only appear on Linus's
machine. My feeling is that some of the more tricksy and problematic global
ring state is actually part of the request state.

I am feeling more confident that we can make intel_context our view of
the hardware state and hang the legacy plumbing off it, and that can be
done afterwards. Though it looks more and more like we need to make the
request as the central object through which we program the hardware.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre