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

Daniel Vetter
2014-08-15T09:38:20Z

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