Re: [PATCH 30/43] drm/i915/bdw: Two-stage execlist submit process

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 15, 2014 at 08:51:22AM +0000, Daniel, Thomas wrote:
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Thursday, August 14, 2014 9:10 PM
> > To: Daniel, Thomas
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 30/43] drm/i915/bdw: Two-stage execlist
> > submit process
> > On Thu, Jul 24, 2014 at 05:04:38PM +0100, Thomas Daniel wrote:
> > > From: Michel Thierry <michel.thierry@intel.com>
> > > +static void execlists_context_unqueue(struct intel_engine_cs *ring) {
> > > +	struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
> > > +	struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
> > > +
> > > +	if (list_empty(&ring->execlist_queue))
> > > +		return;
> > > +
> > > +	/* Try to read in pairs */
> > > +	list_for_each_entry_safe(cursor, tmp, &ring->execlist_queue,
> > > +execlist_link) {
> > 
> > Ok, because checkpatch I've looked at this. Imo open-coding this would be
> > much easier to read i.e.
> > 
> > 	if (!list_empty)
> > 		grab&remove first item;
> > 	if (!list_empty)
> > 		grab&remove 2nd item;
> > 
> > Care to follow up with a patch for that?
> > 
> > Thanks, Daniel
> This needs to be kept as a loop because if there are two consecutive
> requests for the same context they are squashed.  Also the non-squashed
> requests are not removed here (unfortunately the remove is in the next
> patch).

Ok, this sounds like we need to overhaul it anyway for the request
tracking then.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch